Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR comprehensively replaces zccache with soldr (v0.7.4) as the Rust toolchain wrapper across CI workflows and development tooling. macOS and Windows workflows build soldr from source, while Linux workflows install it via script. All cargo invocations are prefixed with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
CLAUDE.md (1)
7-40:⚠️ Potential issue | 🟡 MinorSeparate optional zccache setup from normal build commands.
Line 7 says not to add repo-specific
RUSTC_WRAPPERwiring for normal builds, but Lines 39-40 still presentci/zccache_setup.pyin the main command block. Since that script writesrustc-wrapper = "zccache", make the optional/legacy scope explicit or move it out of the normal command list.📝 Proposed wording update
-# Local zccache setup (optional, configures rustc-wrapper) -uv run python ci/zccache_setup.py +# Optional zccache wrapper-mode only; do not use for standard soldr builds +uv run python ci/zccache_setup.pyBased on learnings, Always use
uv run,soldr, or_cargo/_rustc/_rustfmttrampolines to execute Rust commands. Bare cargo/rustc are blocked by hook.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 7 - 40, The README currently mixes optional zccache setup with normal build commands: move or separate the ci/zccache_setup.py invocation out of the main "Commands" block (or clearly mark it as optional/legacy) and update the wording referencing RUSTC_WRAPPER/zccache so it is explicit that ci/zccache_setup.py configures rustc-wrapper = "zccache" and must not be used for normal builds; ensure the lines showing "./_cargo", "./_rustfmt", uv run python ci/zccache_setup.py and the note about RUSTC_WRAPPER are either relocated to an "Optional zccache setup" section or prefixed with a clear "Optional / Legacy" label.CODEX.md (1)
25-45:⚠️ Potential issue | 🟡 MinorDon’t list zccache setup as a normal approved command.
Line 25 prohibits repo-specific
RUSTC_WRAPPERwiring for standard builds, but Line 45 still appears under “Use these”. Move this to an explicitly optional/legacy wrapper-mode section or remove it from the normal command list.📝 Proposed cleanup
-uv run python ci/zccache_setup.py +# Optional wrapper-mode only; standard builds should use soldr/uv/trampolines above. +# uv run python ci/zccache_setup.pyBased on learnings, Always use
uv run,soldr, or_cargo/_rustc/_rustfmttrampolines to execute Rust commands. Bare cargo/rustc are blocked by hook.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CODEX.md` around lines 25 - 45, The entry "uv run python ci/zccache_setup.py" should not be listed as a normal approved command alongside the canonical trampolines (uv run, soldr, _cargo/_rustc/_rustfmt) because it implies repo-specific RUSTC_WRAPPER wiring; either remove that line from the "Use these" list or move it into a new "optional/legacy wrapper-mode" section and clearly label it as non-standard; update CODEX.md to reference ci/zccache_setup.py only from that optional section and ensure the main list emphasizes using "uv run", "soldr", or the "_cargo/_rustc/_rustfmt" trampolines and that bare cargo/rustc are blocked by hooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/check-macos.yml:
- Around line 27-30: Replace the bare cargo invocation with the UV tool wrapper
to comply with the tool guard policy: find the step that runs "cargo build
--package soldr-cli --release --locked" and change it to run via UV by invoking
"uv run cargo build --package soldr-cli --release --locked" (i.e., wrap the
existing cargo command with "uv run") so the workflow uses the correct Rust
toolchain.
In @.github/workflows/check-windows.yml:
- Around line 27-30: The workflow uses a bare cargo invocation ("cargo build
--package soldr-cli --release --locked"); replace each bare "cargo build ..."
occurrence with an approved Rust-command trampoline (for example prefix with "uv
run --" or use the repository trampolines "_cargo build ...", "_rustc ..." or
the "soldr" wrapper) so the build uses the trampoline instead of calling cargo
directly, and apply the same replacement to the identical occurrences in the
other CI workflow files where the same "cargo build --package soldr-cli
--release --locked" line appears.
In @.github/workflows/template_native_build.yml:
- Around line 97-103: Replace the incorrect invocations that call cargo-zigbuild
with a stray positional "build" argument (e.g., "soldr cargo zigbuild build
--release --target ... -p fbuild-cli" and the fbuild-daemon variant) so they
instead invoke cargo-zigbuild directly with flags (e.g., "soldr cargo zigbuild
--release --target ... -p ..."); remove the extra "build" token from both
zigbuild command occurrences in the workflow (the first pair shown and the
second pair further below) so cargo-zigbuild receives only flags and arguments
it recognizes.
- Around line 59-62: The CI step "Build soldr from source" currently calls the
bare command `cargo build`, which violates the repository's pre-tool hook;
update the step to invoke the wrapper command `soldr cargo build --package
soldr-cli --release --locked` instead so the workflow uses the standard `soldr
cargo` pattern used elsewhere in the file.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 7-40: The README currently mixes optional zccache setup with
normal build commands: move or separate the ci/zccache_setup.py invocation out
of the main "Commands" block (or clearly mark it as optional/legacy) and update
the wording referencing RUSTC_WRAPPER/zccache so it is explicit that
ci/zccache_setup.py configures rustc-wrapper = "zccache" and must not be used
for normal builds; ensure the lines showing "./_cargo", "./_rustfmt", uv run
python ci/zccache_setup.py and the note about RUSTC_WRAPPER are either relocated
to an "Optional zccache setup" section or prefixed with a clear "Optional /
Legacy" label.
In `@CODEX.md`:
- Around line 25-45: The entry "uv run python ci/zccache_setup.py" should not be
listed as a normal approved command alongside the canonical trampolines (uv run,
soldr, _cargo/_rustc/_rustfmt) because it implies repo-specific RUSTC_WRAPPER
wiring; either remove that line from the "Use these" list or move it into a new
"optional/legacy wrapper-mode" section and clearly label it as non-standard;
update CODEX.md to reference ci/zccache_setup.py only from that optional section
and ensure the main list emphasizes using "uv run", "soldr", or the
"_cargo/_rustc/_rustfmt" trampolines and that bare cargo/rustc are blocked by
hooks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8be4a20-848d-4743-881e-61d74cc631bb
📒 Files selected for processing (14)
.github/workflows/check-macos.yml.github/workflows/check-ubuntu.yml.github/workflows/check-windows.yml.github/workflows/docs.yml.github/workflows/fmt.yml.github/workflows/msrv.yml.github/workflows/template_build.yml.github/workflows/template_native_build.ymlCLAUDE.mdCODEX.md_cargoci/README.mdci/dev-tools/README.mdci/trampoline.py
| - name: Build soldr from source | ||
| shell: bash | ||
| working-directory: soldr | ||
| run: cargo build --package soldr-cli --release --locked |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether new workflow changes contain bare cargo invocations that the hook policy should reject.
rg -nP '^\s*run:\s*cargo\b|^\s*cargo\s+\w+' .github/workflows ci -C2Repository: FastLED/fbuild
Length of output: 1166
🏁 Script executed:
cat ci/hooks/tool_guard.py | head -100Repository: FastLED/fbuild
Length of output: 3357
🏁 Script executed:
rg -l "bootstrap|exempt" ci/ --type pyRepository: FastLED/fbuild
Length of output: 40
🏁 Script executed:
cat CLAUDE.md | grep -A5 -B5 "bootstrap\|exempt" || echo "No bootstrap/exempt references found"Repository: FastLED/fbuild
Length of output: 94
Use uv run to wrap the bootstrap cargo command.
The bare cargo build at line 30 violates the tool guard policy. All Rust commands must be invoked through uv run, soldr, or trampolines (_cargo/_rustc/_rustfmt). Replace with uv run cargo build --package soldr-cli --release --locked to ensure the correct Rust toolchain is used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/check-macos.yml around lines 27 - 30, Replace the bare
cargo invocation with the UV tool wrapper to comply with the tool guard policy:
find the step that runs "cargo build --package soldr-cli --release --locked" and
change it to run via UV by invoking "uv run cargo build --package soldr-cli
--release --locked" (i.e., wrap the existing cargo command with "uv run") so the
workflow uses the correct Rust toolchain.
|
Addressed the review feedback in 882fe76. Changes made:
Verification repeated locally:
|
1 participant
Summary
Verification
Notes
Summary by CodeRabbit
Release Notes