chore: remove FFI header generation from pre-commit FFI step#565
chore: remove FFI header generation from pre-commit FFI step#565xdustinface wants to merge 1 commit intov0.42-devfrom
pre-commit FFI step#565Conversation
Headers are generated by `build.rs` via `cbindgen` during `cargo build` and dont need to be tracked in git or verified by `pre-commit`. - Remove `build_ffi_crates()` and header diff checks from `verify_ffi.py` - Gitignore generated headers (`include/` dirs) - Remove `header` target from both FFI Makefiles - Make `key-wallet-ffi/build.rs` panic on `cbindgen` failure to make sure CI fails if there are failures - Move `verify-ffi` from `pre-push` to `pre-commit` stage
📝 WalkthroughWalkthroughThis PR transitions FFI header generation from manual workflow to automatic generation during Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #565 +/- ##
==========================================
Coverage 66.35% 66.35%
==========================================
Files 311 311
Lines 64976 64976
==========================================
+ Hits 43116 43117 +1
+ Misses 21860 21859 -1
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 65-71: The verify-ffi hook (id: verify-ffi, entry:
contrib/verify_ffi.py) was left without a stages key so it runs on every
pre-commit and slows down commits; add a stages entry (for example stages:
[push] or stages: [manual]) under the verify-ffi hook to remove it from the
pre-commit fast path so it only runs on push or when invoked manually, leaving
the existing files pattern and other fields unchanged.
In `@contrib/verify_ffi.py`:
- Around line 56-64: The subprocess.run calls that invoke git (used after
checking docs_result.returncode in contrib/verify_ffi.py) should not rely on
PATH; resolve git once with shutil.which (e.g., git_path = shutil.which("git"))
and reuse that absolute path for both subprocess.run invocations instead of the
literal "git" string, and add a guard to raise/print a clear error if
shutil.which returns None; update the two subprocess.run calls that currently
pass ["git", ...] to use [git_path, ...] (and ensure shutil is imported).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 167b199a-cbc5-4341-bdd7-9d67b38aaa55
📒 Files selected for processing (10)
.gitignore.pre-commit-config.yamlcontrib/verify_ffi.pydash-spv-ffi/CLAUDE.mddash-spv-ffi/Makefiledash-spv-ffi/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.hkey-wallet-ffi/Makefilekey-wallet-ffi/build.rskey-wallet-ffi/include/key_wallet_ffi.h
💤 Files with no reviewable changes (2)
- dash-spv-ffi/dash_spv_ffi.h
- dash-spv-ffi/include/dash_spv_ffi.h
Headers are generated by
build.rsviacbindgenduringcargo buildand dont need to be tracked in git or verified bypre-commit.build_ffi_crates()and header diff checks fromverify_ffi.pyinclude/dirs)headertarget from both FFI Makefileskey-wallet-ffi/build.rspanic oncbindgenfailure to make sure CI fails if there are failuresverify-ffifrompre-pushtopre-commitstageSummary by CodeRabbit
Chores
Documentation