Split `cargo check` matrix to individual targets. Avoid cache poisoning by ShaharNaveh · Pull Request #7540 · RustPython/RustPython
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4b694ed5-7013-4d21-b1e2-20d1556c7d9e
📒 Files selected for processing (1)
.github/workflows/ci.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yaml
📝 Walkthrough
Walkthrough
Restructures the CI cargo_check job to run per-target matrix entries, adds explicit env vars, replaces Swatinem/rust-cache with actions/cache restore/save, switches from dtolnay/rust-toolchain to manual rustup installs, and adjusts Android NDK and caching conditions.
Changes
| Cohort / File(s) | Summary |
|---|---|
CI Workflow .github/workflows/ci.yaml |
Reworks cargo_check job: set CARGO_INCREMENTAL=0 and CARGO_TERM_COLOR=always; change matrix from multi-target loop to per-target rows; replace Swatinem/rust-cache with actions/cache/restore and actions/cache/save using keys scoped by runner.os and matrix.target (save keyed also on Cargo.toml, Cargo.lock, github.sha); switch toolchain setup from dtolnay/rust-toolchain to rustup toolchain install stable --target "${{ matrix.target }}"; adjust Android NDK setup condition to matrix.target == 'aarch64-linux-android'; run a single cargo check --target "${{ matrix.target }}" per job. |
Sequence Diagram(s)
(omitted)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- Restructure CI with matrix approach and multi-feature support #7376: Similar CI workflow changes reorganizing the cargo_check matrix and cargo invocation.
- Use matrix for
cargo check#7388: Also moves cargo_check to a per-target matrix and adjusts caching/toolchain steps. - Remove duplicate
cargo check --target x86_64-unknown-freebsd#7377: Related CI adjustments affecting target handling and toolchain setup.
Suggested reviewers
- youknowone
Poem
🐰 I hopped through YAML, tidy and bright,
Caches snug, targets set right.
Per-target steps now bound and neat,
Rust checks hop forward, light on their feet.
🥕🦀
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately reflects the main changes in the pull request: splitting the cargo check matrix to individual targets and implementing cache poisoning prevention measures. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.