Repository navigation
Added Rust coding standards to CLAUDE.md - #215
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughCLAUDE.md is updated with a new "Rust Coding Standards" section detailing repository-wide conventions for functional programming style, error handling, async/concurrency patterns, tracing-based logging, and general practices including testing and dependency management. ChangesRust Coding Standards Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hidi 8000 ng this comment.
🧹 Nitpick comments (1)
CLAUDE.md (1)
144-144: ⚡ Quick winUnify clippy/test command guidance across the document.
This line conflicts with the earlier Development Commands section (
cargo clippy --allow-dirty --fix ...). Please make one canonical command set (or explicitly split “local auto-fix” vs “CI/strict check”) so agents don’t follow conflicting workflows.Based on learnings: Run
cargo clippy --all-targets -- -D warningsandcargo testfor every Rust change; justify any#[allow(...)]with a comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` at line 144, Update CLAUDE.md to remove the conflicting clippy/test guidance by choosing a single canonical command set (or explicitly labeling two workflows "local auto-fix" vs "CI/strict") and make both occurrences consistent: replace the earlier `cargo clippy --allow-dirty --fix ...` guidance and the later `cargo clippy --all-targets -- -D warnings` line so they either both document the local auto-fix flow (e.g., `cargo clippy --allow-dirty --fix --all-targets` plus `cargo test`) and a separate CI/strict command (`cargo clippy --all-targets -- -D warnings` and `cargo test`), or consolidate to the single strict pair `cargo clippy --all-targets -- -D warnings` and `cargo test`; also keep the instruction to justify any `#[allow(...)]` with a comment and ensure both places mention that justification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@CLAUDE.md`:
- Line 144: Update CLAUDE.md to remove the conflicting clippy/test guidance by
choosing a single canonical command set (or explicitly labeling two workflows
"local auto-fix" vs "CI/strict") and make both occurrences consistent: replace
the earlier `cargo clippy --allow-dirty --fix ...` guidance and the later `cargo
clippy --all-targets -- -D warnings` line so they either both document the local
auto-fix flow (e.g., `cargo clippy --allow-dirty --fix --all-targets` plus
`cargo test`) and a separate CI/strict command (`cargo clippy --all-targets --
-D warnings` and `cargo test`), or consolidate to the single strict pair `cargo
clippy --all-targets -- -D warnings` and `cargo test`; also keep the instruction
to justify any `#[allow(...)]` with a comment and ensure both places mention
that justification.
|
|
||
| ### Error Handling | ||
|
|
||
| - Library crates: typed error enum via thiserror. Variants are distinct failure modes; carry source via #[from] / #[source]. |
There was a problem hiding this comment.
thiserror over manual impls? Not sure whats right here but ive always leaned to the latter 😅
There was a problem hiding this comment.
The #[from] macro is really nice in thiserror. I don't feel that strongly about using it though. Is there a reason we want to avoid it?
There was a problem hiding this comment.
Removed it for now. Better to not couple that sort of decision with this.
|
Just a comment, no action needed. At some point we probably want to break out some the sections in this doc into reusable, referenceable fragments. This way we can be a little more precise about when we inject them them into the context window. On top of that reusability is just good no matter what. |
a96331a to
26b9187
Compare
Added some Rust coding standards for agents to CLAUDE.md to see if we can improve the quality of the Rust code it generates.
Summary by CodeRabbit