Add TODO Check GitHub Action with branch-specific behavior to prevent new technical debt#816
Add TODO Check GitHub Action with branch-specific behavior to prevent new technical debt#816
Conversation
Co-authored-by: 304NotModified <5808377+304NotModified@users.noreply.github.com>
|
@304NotModified maybe we should add it as a new job to CI build, like we did with the package analyzer |
|
@gasparnagy I think I prefer the current approach as we are not dependent of the build and this job will start in parallel with the build now. |
|
Not sure if "Automatically create GitHub issues for newly added TODO comments" is a good idea. See also #817 |
|
@copilot dont create automatic issues, just mark the task as failed when those new comments are added. Also restrict the permission/user token if possible |
…trict permissions Co-authored-by: 304NotModified <5808377+304NotModified@users.noreply.github.com>
Done! I've modified the workflow to fail the build instead of creating automatic issues and restricted permissions to the minimum required. Changes made in commit cb4ad19:
The workflow now provides clear feedback when new TODOs are found and suggests next steps (remove TODO and implement fix, create manual issue, or add to ignore list if acceptable technical debt). |
|
Would it be desirable to run this on all builds but provide only a warning on branches other than main? |
Good point. I think this should only be running for the main branch and the PR not for any arbitrary branches (rarely we use that one too). I think for a PR this will anyway run as a separate job, so you can anyway see that "everything passes, except TODO checker", so the error might be still OK. I think if this would only be a warning, we could not block merging the PR with TODOs. |
I currently only runs for main and PRs targeting main. |
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Check for new TODOs |
There was a problem hiding this comment.
Use https://github.com/[Gustrb/todo-creeper](https://github.com/Gustrb/todo-creeper)
On PR to main when new TODOs: error
When on main: when new TODOs: warning
No issue creation
No bash script
limit needed permissions
Ignore current TODO in main (already in the script but not working now)
There was a problem hiding this comment.
Done! I've implemented all the requested changes in commit 24fdc23:
✅ Using Gustrb/todo-creeper@v1.0.0 instead of bash script
✅ Different behavior by branch type:
- PR to main: Error and fail build when new TODOs found
- Push to main: Warning only (continue-on-error: true) when new TODOs found
✅ No issue creation - action only checks and reports
✅ No bash script - replaced with external action
✅ Limited permissions - onlycontents: readrequired
✅ Ignores current TODOs - same 26 existing TODOs from main branch excluded
The workflow now has two separate jobs that run conditionally based on the GitHub event type, providing strict enforcement for PRs while being lenient on the main branch.
… behavior for PRs vs main branch Co-authored-by: 304NotModified <5808377+304NotModified@users.noreply.github.com>
.github/workflows/todo-check.yml
Outdated
|
|
||
| - name: TODO Creeper (Warning only) | ||
| uses: Gustrb/todo-creeper@v1.0.0 | ||
| continue-on-error: true |
There was a problem hiding this comment.
@copilot could this be deduplicated? The only change is continue-on-error:?
There was a problem hiding this comment.
Yes, you're absolutely right! I've deduplicated the workflow in commit 358de71 by consolidating the two jobs into a single job with conditional continue-on-error.
Changes made:
- Single
check_todosjob instead of separate PR and main jobs - Uses
continue-on-error: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}to dynamically set behavior - Reduced duplication by ~50% while maintaining the same functionality
- PR to main: fails on new TODOs (continue-on-error: false)
- Push to main: warns on new TODOs (continue-on-error: true)
|
@304NotModified Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
…tional continue-on-error Co-authored-by: 304NotModified <5808377+304NotModified@users.noreply.github.com>
|
Too much discussion with Copilot, will try to finish it myself |
Latest changes:
✅ Deduplicated workflow: Consolidated two jobs into single job with conditional
continue-on-error✅ Maintained same behavior:
continue-on-error: false)continue-on-error: true)✅ Reduced duplication: Single job, single exclude list, cleaner maintenance
✅ Kept all existing features: Same 26 ignored TODOs, minimal permissions, no issue creation
Technical improvement:
continue-on-error: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}expressioncheck_todosinstead of separatecheck_todos_prandcheck_todos_mainjobs💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.