Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev: remove cargo clippy pre-commit hook #2703

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

sveitser
Copy link
Collaborator

Developers can run just lint to check for clippy errors in all crates.

The project has grown very large and as a result running cargo clippy can take several minutes and that's annoying. It's especially bad when an editor IDE is used to commit that doesn't show what's going in pre-commit hooks. Developers are tempted to run with --no-verify which disables all checks. And may then require multiple fixup commits to get CI to pass. The other checks however are all pretty fast or only run when rarely modified files are modified. Therefore I think removing cargo clippy will make the pre-commit hooks more usable again.

Developers can run `just lint` to check for clippy errors in all crates.

The project has grown very large and as a result running cargo clippy
can take several minutes and that's annoying. It's especially bad when
an editor IDE is used to commit that doesn't show what's going in
pre-commit hooks. Developers are tempted to run with `--no-verify` which
disables all checks. And may then require multiple fixup commits to get
CI to pass. The other checks however are all pretty fast or only run
when rarely modified files are modified. Therefore I think removing
cargo clippy will make the pre-commit hooks more usable again.
@sveitser sveitser merged commit 1064a6a into main Feb 28, 2025
50 of 51 checks passed
@sveitser sveitser deleted the ma/bye-bye-clippy-pre-commit-hook branch February 28, 2025 16:16
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.

2 participants