Skip to content

chore: clean up stale TODO comments and update issue references#2736

Merged
pbeza merged 24 commits intomainfrom
2732-ci-detect-todo-comments-referencing-closed-github-issues
Apr 8, 2026
Merged

chore: clean up stale TODO comments and update issue references#2736
pbeza merged 24 commits intomainfrom
2732-ci-detect-todo-comments-referencing-closed-github-issues

Conversation

@pbeza
Copy link
Copy Markdown
Contributor

@pbeza pbeza commented Apr 7, 2026

Clean up stale TODOs referencing closed GitHub issues and update references where work moved to new issues.

Split from the CI check work — the script and workflow changes are in #2749. This PR focuses on the TODO comment cleanup.

Closes #2750

@pbeza pbeza linked an issue Apr 7, 2026 that may be closed by this pull request
5 tasks
@pbeza pbeza marked this pull request as ready for review April 7, 2026 10:55
Copilot AI review requested due to automatic review settings April 7, 2026 10:55
@claude
Copy link
Copy Markdown

claude bot commented Apr 7, 2026

Code Review

Clean housekeeping PR. The new check-todo-closed-issues.sh script is well-structured: it batches GitHub API calls into a single GraphQL query, handles errors gracefully, and runs as non-blocking in CI (continue-on-error: true). The stale TODO removals look correct — context was preserved where needed (e.g., the migration-service doc now links to an existing backup-service-attestation-data.md), and the re-tagged TODO(#2102) in run.rs correctly points to an open issue.

No blocking issues found.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new CI-side audit to detect TODO(#NNN) comments that reference already-closed GitHub issues, and cleans up/updates a set of stale TODOs across the repo to reduce misleading technical-debt markers.

Changes:

  • Add scripts/check-todo-closed-issues.sh to query GitHub and fail on TODOs pointing to closed issues (intended as a non-blocking CI warning).
  • Wire the check into CI (non-blocking) and expose it locally via cargo make todo-closed-issues.
  • Remove/update multiple stale TODOs (and a few related formatting-only tweaks) across Rust, docs, config, and workflow files.

Reviewed changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scripts/check-todo-closed-issues.sh New script that finds TODO(#NNN) references and checks issue state via GitHub GraphQL.
.github/workflows/ci.yml Adds a non-blocking CI step to run the new TODO closed-issue check.
Makefile.toml Adds todo-closed-issues cargo-make task for local execution.
pytest/tests/test_lost_assets.py Removes a stale TODO reference.
docs/migration-service.md Replaces a TODO reference with a concrete doc link.
docs/chain-gateway-design.md Removes stale TODO annotations from a design snippet.
crates/test-parallel-contract/src/lib.rs Removes a stale TODO comment (minor whitespace tweak).
crates/node/src/run.rs Updates TODO issue reference.
crates/node/src/providers/verify_foreign_tx.rs Removes a stale TODO comment.
crates/node/src/providers/eddsa/key_resharing.rs Removes stale TODO and reflows method call formatting.
crates/node/src/providers/eddsa/key_generation.rs Removes stale TODO and reflows method call formatting.
crates/node/src/providers/ecdsa/key_resharing.rs Removes stale TODO and reflows method call formatting.
crates/node/src/providers/ecdsa/key_generation.rs Removes stale TODO and reflows method call formatting.
crates/node/src/providers/ckd/key_resharing.rs Removes stale TODO and reflows method call formatting.
crates/node/src/providers/ckd/key_generation.rs Removes stale TODO and reflows method call formatting.
crates/node/src/protocol.rs Removes stale TODO commentary.
crates/node/src/network.rs Removes stale TODO in a test/dummy implementation.
crates/node/src/indexer/handler.rs Removes stale TODO comments in handler logic.
crates/node/src/indexer.rs Removes stale TODO comments in RPC/indexer structures.
crates/node/src/config.rs Removes stale TODO comments around secrets/respond config.
crates/node/src/cli.rs Removes stale TODO from CLI arg docs.
crates/node/src/assets.rs Rewords TODO doc comment into a note.
crates/contract/tests/sandbox/vote.rs Removes stale TODO at file header.
crates/contract/tests/sandbox/utils/consts.rs Rewords/removes stale TODOs in gas/deposit constants docs.
crates/contract/src/tee/measurements.rs Removes stale TODO in migration helper docs.
crates/contract/README.md Removes stale TODO from setup steps.
Cargo.toml Removes a stale TODO and applies formatting-only changes.
.gitignore Removes a stale TODO comment.
.github/dependabot.yml Removes stale TODO notes / adjusts ignore list entries accordingly.
Comments suppressed due to low confidence (1)

.github/workflows/ci.yml:607

  • In CI this step will likely run unauthenticated: gh api graphql requires a token (typically via GH_TOKEN/GITHUB_TOKEN env). Consider setting GH_TOKEN: ${{ github.token }} for this step (or job-wide) and updating fast-ci-checks job permissions to include issues: read, otherwise the check may consistently fail and show a permanent yellow warning.

    steps:
      - name: Checkout repository
        uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/check-todo-closed-issues.sh Outdated
Comment thread scripts/check-todo-closed-issues.sh Outdated
@pbeza
Copy link
Copy Markdown
Contributor Author

pbeza commented Apr 7, 2026

This check is not part of check-all-fast because it can't be a blocking check — someone closing an issue as "not planned" would suddenly break CI on unrelated PRs. It also requires GitHub API access (gh authenticated), unlike other fast checks.

pbeza added 2 commits April 7, 2026 13:05
The todo-format check was catching "todo" in the filename and script
contents of check-todo-closed-issues.sh. Extract excluded patterns
into a constant array for maintainability.
grep -z is a GNU extension not available on macOS/BSD grep.
Use git ls-files (without -z) piped to grep -v instead.
@pbeza pbeza requested review from DSharifi, gilcu3 and netrome April 7, 2026 11:11
DSharifi
DSharifi previously approved these changes Apr 7, 2026
@pbeza pbeza requested a review from Copilot April 7, 2026 18:17
@pbeza
Copy link
Copy Markdown
Contributor Author

pbeza commented Apr 7, 2026

@claude review

@claude
Copy link
Copy Markdown

claude bot commented Apr 7, 2026

Code Review

Clean housekeeping PR. No critical issues found.

Minor (non-blocking): In check-todo-closed-issues.sh line 62, the grep pattern uses unescaped parentheses which are regex metacharacters. Consider grep -Fn for exact fixed-string matching. Not blocking since false positives are extremely unlikely.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/check-todo-format.sh Outdated
Comment thread scripts/check-todo-format.sh Outdated
Comment thread scripts/check-todo-closed-issues.sh Outdated
Comment thread scripts/check-todo-closed-issues.sh Outdated
Comment thread scripts/check-todo-closed-issues.sh Outdated
@pbeza pbeza requested a review from DSharifi April 7, 2026 18:51
@claude
Copy link
Copy Markdown

claude bot commented Apr 8, 2026

PR title type suggestion: This PR primarily adds new CI infrastructure (workflow and check scripts), so the type prefix should probably be ci: instead of chore:.

Suggested title: ci: add check for TODOs referencing closed issues and clean up stale TODOs

@pbeza pbeza changed the title chore: add CI check for TODOs referencing closed issues and clean up stale TODOs chore: clean up stale TODO comments and update issue references Apr 8, 2026
gilcu3
gilcu3 previously approved these changes Apr 8, 2026
Copy link
Copy Markdown
Contributor

@gilcu3 gilcu3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Comment thread crates/test-parallel-contract/src/lib.rs
Comment thread pytest/tests/test_lost_assets.py
@pbeza pbeza added this pull request to the merge queue Apr 8, 2026
Merged via the queue into main with commit 6a0584b Apr 8, 2026
26 checks passed
@pbeza pbeza deleted the 2732-ci-detect-todo-comments-referencing-closed-github-issues branch April 8, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up stale TODO comments referencing closed issues CI: ensure PRs that close issues also remove associated TODOs

5 participants