chore: add pre-commit hooks for code quality checks#3194
chore: add pre-commit hooks for code quality checks#3194QuantumExplorer merged 4 commits intov3.1-devfrom
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (git)
participant Pre as pre-commit
participant Fast as Fast Hooks
participant Slow as Slow Hooks
participant Tools as Tools (gitleaks, cargo-fmt, check-* etc.)
Dev->>Pre: git commit / git push
Pre->>Fast: run commit-stage hooks
Fast->>Tools: run formatting, lint, checks, secret scan
Tools-->>Fast: results (pass/fail)
alt commit successful
Dev->>Pre: git push (or manual)
Pre->>Slow: run push/manual hooks
Slow->>Tools: run cargo-clippy (workspace)
Tools-->>Slow: results (pass/fail)
end
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.pre-commit-config.yaml (2)
57-66: Consider adding-- -D warningsto fail on clippy warnings.The clippy hook runs without treating warnings as errors. For a pre-push check, you may want stricter enforcement to catch issues before they reach CI.
♻️ Proposed enhancement for stricter clippy checks
- id: clippy name: clippy (workspace) description: Run clippy on entire workspace - entry: cargo clippy --workspace + entry: cargo clippy --workspace -- -D warnings language: system types: [rust] pass_filenames: false stages: [pre-push, manual]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 57 - 66, The clippy hook (id: clippy) currently runs with entry "cargo clippy --workspace" and doesn't treat warnings as errors; update the hook's entry to run clippy with strictness by appending the cargo flag and rustc args (e.g., change the entry for the clippy hook to include "-- -D warnings") so clippy warnings are treated as failures during pre-push.
32-41: Thenode_modules/exclusion is redundant with the global exclude pattern.The global exclude pattern (lines 74–75) already excludes
node_modules/and.*/node_modules/, making the exclusion in the typos hook (lines 37–41) unnecessary. Remove it to keep the configuration DRY.Additionally, consider whether
--write-changesis the desired behavior. While convenient, this flag automatically modifies files when issues are found, which may be surprising for developers who prefer to review suggested changes first. Some teams prefer--diffto show changes without applying them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 32 - 41, Remove the redundant node_modules/ exclusion from the typos hook block (repo: https://github.com/crate-ci/typos, hook id: typos) so the hook relies on the global exclude pattern instead of repeating .*node_modules/; also evaluate the args for this hook (currently '--write-changes') and, if you prefer reviewers to inspect edits before applying them, replace '--write-changes' with '--diff' (or remove the flag) to avoid auto-applying fixes._typos.toml (1)
4-6: Consider expanding the comment or word list.The comment mentions "Blockchain/crypto terms" but "datas" is a general non-standard pluralization (common in various technical contexts) rather than a blockchain-specific term. Consider either:
- Updating the comment to be more general (e.g., "Technical terms that are not typos")
- Adding actual blockchain/crypto terms that might trigger false positives (e.g.,
merkle,secp,ecdsa,keccak)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@_typos.toml` around lines 4 - 6, The comment above [default.extend-words] says "Blockchain/crypto terms" but the only entry is "datas" (key: datas), which is a general non-standard plural rather than a crypto-specific term; update the comment to a more general description (e.g., "Technical terms that are not typos") or replace/augment the list with real blockchain/crypto tokens (e.g., merkle, secp, ecdsa, keccak, gas, nonce) while keeping or moving "datas" as needed under [default.extend-words].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@_typos.toml`:
- Around line 4-6: The comment above [default.extend-words] says
"Blockchain/crypto terms" but the only entry is "datas" (key: datas), which is a
general non-standard plural rather than a crypto-specific term; update the
comment to a more general description (e.g., "Technical terms that are not
typos") or replace/augment the list with real blockchain/crypto tokens (e.g.,
merkle, secp, ecdsa, keccak, gas, nonce) while keeping or moving "datas" as
needed under [default.extend-words].
In @.pre-commit-config.yaml:
- Around line 57-66: The clippy hook (id: clippy) currently runs with entry
"cargo clippy --workspace" and doesn't treat warnings as errors; update the
hook's entry to run clippy with strictness by appending the cargo flag and rustc
args (e.g., change the entry for the clippy hook to include "-- -D warnings") so
clippy warnings are treated as failures during pre-push.
- Around line 32-41: Remove the redundant node_modules/ exclusion from the typos
hook block (repo: https://github.com/crate-ci/typos, hook id: typos) so the hook
relies on the global exclude pattern instead of repeating .*node_modules/; also
evaluate the args for this hook (currently '--write-changes') and, if you prefer
reviewers to inspect edits before applying them, replace '--write-changes' with
'--diff' (or remove the flag) to avoid auto-applying fixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af767f17-120f-44c2-ba79-e58982d62d82
📒 Files selected for processing (2)
.pre-commit-config.yaml_typos.toml
✅ gRPC Query Coverage Report |
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "bdab166b0da2de1ce7c96a853ea79cc3d44a6f6023bb69d4b09c451594caa22c"
)Xcode manual integration:
|
387ea2c to
1911d08
Compare
Add pre-commit configuration modeled after grovedb's setup: On commit (fast): - Trailing whitespace, EOF fixer, YAML/JSON/TOML validation - Merge conflict detection, line ending normalization - Large file check (1MB limit), case conflict detection - Typos spell checker with blockchain-aware config - cargo fmt (Rust formatting) On push (slow): - cargo clippy (workspace-wide linting) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Typos were already fixed in a separate PR. Remove the typos hook and its _typos.toml config to keep pre-commit checks lightweight. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e690cdc to
34a0d33
Compare
Exclude test vectors, supporting files, protobuf generated code, fixtures, lock files, and other autogenerated content from hooks. Exclude JSONC files (tsconfig, devcontainer) from check-json. Remove end-of-file-fixer (conflicts with cargo fmt on empty files) and shebang checks (pre-existing issues to fix separately). Add gitleaks hook for secret detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
34a0d33 to
2d378cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 56-63: Pre-commit's clippy hook (id: clippy, entry: cargo clippy
--workspace) is less strict than CI; update the hook entry to match CI by
running cargo clippy --workspace --all-features -D warnings so local runs fail
on warnings and lint feature-gated code too, leaving out --locked and --no-deps
as noted.
| - id: clippy | ||
| name: clippy (workspace) | ||
| description: Run clippy on entire workspace | ||
| entry: cargo clippy --workspace | ||
| language: system | ||
| types: [rust] | ||
| pass_filenames: false | ||
| stages: [pre-push, manual] |
There was a problem hiding this comment.
Clippy flags should match CI to prevent surprise failures.
The pre-commit clippy hook uses only cargo clippy --workspace, but CI runs with stricter flags including --all-features and -D warnings. This means code can pass the pre-commit hook but fail CI because:
-D warningsis missing: warnings don't fail locally but are errors in CI--all-featuresis missing: feature-gated code isn't linted locally
🔧 Proposed fix to align with CI
- id: clippy
name: clippy (workspace)
description: Run clippy on entire workspace
- entry: cargo clippy --workspace
+ entry: cargo clippy --workspace --all-features -- -D warnings
language: system
types: [rust]
pass_filenames: false
stages: [pre-push, manual]Note: --locked and --no-deps from CI are omitted intentionally—--locked can cause issues if Cargo.lock drifts locally, and --no-deps is the default behavior when not using -D warnings on dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.pre-commit-config.yaml around lines 56 - 63, Pre-commit's clippy hook (id:
clippy, entry: cargo clippy --workspace) is less strict than CI; update the hook
entry to match CI by running cargo clippy --workspace --all-features -D warnings
so local runs fail on warnings and lint feature-gated code too, leaving out
--locked and --no-deps as noted.
Add --all-features and -D warnings to clippy hook to match CI configuration in tests-rs-package.yml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ce80fac to
43dde2b
Compare
Issue being fixed or feature implemented
No pre-commit hooks exist in the platform repo, allowing formatting issues, invalid config files, and merge conflicts to slip into commits.
What was done?
Added
.pre-commit-config.yamlwith hooks modeled after the grovedb repo setup.On commit (fast checks):
trailing-whitespace— trim trailing whitespace (excludes markdown)check-yaml— validate YAML syntaxcheck-json— validate JSON syntax (excludes JSONC files like tsconfig, devcontainer)check-toml— validate TOML syntaxcheck-merge-conflict— detect merge conflict markersmixed-line-ending— enforce LF line endingscheck-added-large-files— block files over 1MBcheck-case-conflict— detect case-insensitive filename conflictscargo fmt— format Rust codeOn push (slow checks):
cargo clippy— lint entire Rust workspaceGlobal exclusions for generated/autogenerated content:
_pb.js,_pb_service.js,.pbrpc.h).yarn/,node_modules/,CHANGELOG.mdSetup
How has this been tested?
pre-commit run --all-filespasses cleanly on current v3.1-dev (after whitespace cleanup in chore: fix trailing whitespace and missing final newlines #3196)Breaking changes
None. Pre-commit hooks are opt-in per developer.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit