PR 3: Add pre-commit hooks for formatting, linting, and tests#111
PR 3: Add pre-commit hooks for formatting, linting, and tests#111
Conversation
Add scripts/pre-commit hook that runs cargo fmt --check and cargo clippy on staged Rust files before allowing commits. This catches lint and formatting issues locally before they hit CI. Add Makefile targets: - make install-hooks: installs the pre-commit hook - make check: runs lint + test as a single command
scripts/pre-commit
Outdated
| # Run tests | ||
| echo " Running tests..." | ||
| if ! cargo test; then | ||
| echo "" | ||
| echo "Tests failed. Fix the failing tests above." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Maybe a philosophical debate but pre-commit feels like a weird spot to run tests? Keeping this focused on format/lints feels more optimal; tests aren't instant
There was a problem hiding this comment.
thats valid, I'll remove it. I think it is potentially worth adding to projects that are mainly developed using AI but im not sure if that is this one. thanks!
scripts/pre-commit
Outdated
| echo " Checking formatting..." | ||
| if ! cargo fmt -- --check; then | ||
| echo "" | ||
| echo "Formatting check failed. Run 'cargo fmt' to fix." | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
Any reason not to rely on make lint, which runs formatting check and clippy already?
There was a problem hiding this comment.
no reason, i wasn't aware of that standard (new to rust) will make the update
Makefile
Outdated
| .PHONY: check | ||
| check: lint test | ||
| @echo "All checks passed." |
There was a problem hiding this comment.
check is a bit vague, and I don't see this new make target used anywhere in your pre-commit script. Let's remove it?
Co-authored-by: Carey Janecka <1709621+figitaki@users.noreply.github.com>
3 participants
Summary
scripts/pre-commit) that runscargo fmt --check,cargo clippy, andcargo teston staged Rust filesmake install-hookstarget for easy setup andmake checkto run all checks manuallyTest plan
make install-hooksand verify hook is installedmake checkto verify all checks pass