Repository navigation
PR-10: docs/CI: README macOS manual + rollup automation (no tests) - #101
Conversation
…n tips; docs: log in plan + decision log
Summary by CodeRabbit
WalkthroughAdded unit tests for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
README.mdis excluded by!*.md
📒 Files selected for processing (3)
crates/rmg-core/src/snapshot.rs(1 hunks)docs/decision-log.md(1 hunks)docs/execution-plan.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security Audit
🔇 Additional comments (4)
crates/rmg-core/src/snapshot.rs (2)
197-219: Excellent independent verification pattern.This test correctly constructs the expected header bytes independently of the function under test, then verifies equivalence. The spot-checks for little-endian encoding (lines 217-218) provide valuable regression protection beyond the hash equality assertion.
The use of
blake3::hashon the full byte buffer (vs. incrementalHasher::updatein the production code) is appropriate—it confirms that the incremental hashing incompute_commit_hashproduces the same result as a single-pass hash, which is exactly what you want to validate.
221-229: Critical determinism property validated.This test confirms that parent ordering is reflected in the commit hash, which is essential for deterministic merge commit identification. Clean and focused.
docs/decision-log.md (1)
35-36: Documentation entries are clear and well-structured.Both entries correctly follow the established table format, provide appropriate context, and accurately scope their respective changes (tests-only for PR-09, docs-only for PR-10). The consequences are appropriately noted.
docs/execution-plan.md (1)
97-107: Execution plan entries accurately document the changes.Both PR-09 and PR-10 entries are well-structured and follow the established format. The PR-09 description precisely matches the test code added to
snapshot.rs(header bytes construction, hash equivalence, LE encoding verification). The PR-10 description mentions README changes for CI tips that are not included in the files under review.Note: The README.md changes described in the PR-10 section (lines 104-106) are not present in the provided review files. This reinforces the inconsistency flagged earlier—if this PR is supposed to include README updates, those files should be part of the review.
…arkdown in echo-total - pre-commit: trigger rollup when docs/**.md changes (exclude rollup); abort commit if rollup changed to preserve index - generator: include subdirectories (e.g., docs/guide), deterministic LC_ALL=C sort; stable header (no timestamp/SHA) - docs: log automation in execution plan + decision log
- Keep PR-10 strictly docs/CI/tooling; tests remain in PR-09 (echo/pr-09-blake3-header-tests) - Log decision in execution-plan + decision-log (no runtime change) - Regenerate docs/echo-total.md
Scope hygiene: this PR is strictly docs/CI/tooling.
Changes
Note
Rationale