Forbid TODOs in the codebase #1921
Conversation
| - name: Forbid TODOs | ||
| run: ./scripts/forbid-todo.sh |
There was a problem hiding this comment.
I added to test workflow rather than pre-commit because if someone has our pre-commit hooks installed locally, we don't want to forbid them from commiting a TODO; we just want to ensure it doesn't get merged
There was a problem hiding this comment.
You could still commit skipping the pre-commit hook? i.e. git commit --no-verify
There was a problem hiding this comment.
We could, but I feel that fundamentally it's not a pre-commit requirement. It's a pre-merge requirement. Committing unfinished stuff is totally fine 🤷♂️
| @@ -32,6 +32,7 @@ repos: | |||
| exclude_types: [svg] | |||
| exclude: patches/.*\.patch | |||
| - id: fix-byte-order-marker | |||
| - id: forbid-submodules | |||
There was a problem hiding this comment.
Found this one while looking for a pre-commit hook; seemed worth having
|
|
||
| # Find the string 'TODO' in all files tracked by git, excluding | ||
| # this file | ||
| TODOS_FOUND=$(git grep --color=always -nw TODO -- ':!scripts/forbid-todo.sh' || true) |
There was a problem hiding this comment.
Decided to just grep all files rather than try to find lint rules for different languages, to make sure we cover everything
- Fixes cursorless-dev#1920 Here's a screenshot of a failed run: <img width="1218" alt="image" src="https://github.com/cursorless-dev/cursorless/assets/755842/7b12179c-0582-4cf8-ba7d-59ef4a2e453e"> ## Checklist - [-] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [-] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [-] I have not broken the cheatsheet
Here's a screenshot of a failed run:
Checklist