Skip to content

Document local Rust checks - #56

Merged
kriszyp merged 6 commits into
mainfrom
docs/symphony-rust-local-checks
Oct 15, 2026
Merged

kriszyp merged 6 commits into
mainfrom
docs/symphony-rust-local-checks

Conversation

@kriszyp

@kriszyp kriszyp commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Update CLAUDE.md Testing to require cargo test, cargo clippy --all-targets, and cargo fmt --check before pushing Rust changes, and point Conventions at the all-target Clippy command. Testing also records the pinned toolchain and PATH requirements, the addon rebuild before Node tests, and the Linux checks required before macOS pushes.

For the human reviewer

  1. Requirement and layer. This guidance is warranted because npm test does not run the Rust checks CI enforces. I kept the checks in CLAUDE.md instead of adding a package script or pre-push hook, which would exceed the requested documentation-only scope. That choice is easy to revise, but it relies on contributors following the instructions; CI still catches misses.
  2. Platform check. macOS test and Clippy runs do not compile the Linux-only branches CI checks, so the instructions require running both commands in Linux before pushing from macOS. The existing Docker images are linked from the README, and cargo fmt --check works on either platform. This adds a Linux environment step but catches platform-specific failures before PR CI.
  3. Rustup fallback. I kept rustup toolchain install without a toolchain argument: rustup 1.29.1 accepts that form and installs the active toolchain, which this repo defines in rust-toolchain.toml. The Rustup 1.28.0 release notes document the no-argument behavior; rustup show reports the active toolchain but does not install it.

Verification

  • From the repository root, cargo --version reported 1.94.1.
  • cargo test: 144 passed, 0 failed.
  • cargo clippy --all-targets: passed.
  • cargo fmt --check: passed.
  • git diff --check: passed.
  • npm run build:debug and npm test were not run because this change is documentation-only; no source or generated addon code changed.

Complexity: easy

🤖 Generated with OpenAI Codex

Review-Coverage: authored=codex; ran=gemini,claude; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse,cursor-composer; rounds=6; full=1 @ bd48172

Human-Review-Need: 3 (decisions: local-linux-before-push, contributor-guidance-layer) @ bd48172

kriszyp and others added 4 commits October 2, 2026 09:01
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: symphony-claude-md-cargo-test-note
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: symphony-claude-md-cargo-test-note
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: symphony-claude-md-cargo-test-note
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: symphony-claude-md-cargo-test-note

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates CLAUDE.md with detailed instructions on Rust toolchain setup, local testing, and linting guidelines. The review feedback points out two issues in the documentation: a command error where running rustup toolchain install without arguments fails (suggesting rustup show instead), and a factual inaccuracy regarding how macOS handles Linux-specific compilation branches.

Comment thread CLAUDE.md
Comment thread CLAUDE.md Outdated
kriszyp and others added 2 commits October 2, 2026 09:42
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: symphony-claude-md-cargo-test-note
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: symphony-claude-md-cargo-test-note
@kriszyp
kriszyp marked this pull request as ready for review October 15, 2026 04:51
@kriszyp
kriszyp merged commit e7e5dfc into main Oct 15, 2026
11 checks passed
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.

1 participant