Skip to content

ci: split monolithic CI into parallel jobs#383

Merged
Bisht13 merged 3 commits into
mainfrom
px/split-ci-parallel-jobs
Mar 25, 2026
Merged

ci: split monolithic CI into parallel jobs#383
Bisht13 merged 3 commits into
mainfrom
px/split-ci-parallel-jobs

Conversation

@Bisht13
Copy link
Copy Markdown
Collaborator

@Bisht13 Bisht13 commented Mar 25, 2026

Summary

  • Splits the single build_and_test CI job into separate parallel jobs: fmt, clippy, build+test, and documentation — matching the upstream whir CI pattern
  • Lightweight checks (fmt, typos, unused deps) no longer wait behind a full build, giving faster feedback on failures
  • All provekit-specific settings preserved (ARM runner, nightly-2026-03-04 toolchain, moonrepo/setup-rust, concurrency groups, doc tests)

Jobs

Job Runner Purpose
build_and_test ubuntu-24.04-arm Build + unit/integration tests + doc tests
cargo_clippy ubuntu-24.04-arm Lint check with all features
cargo_fmt ubuntu-24.04-arm Format check
cargo_doc ubuntu-24.04-arm Documentation build with -D warnings
unused_deps ubuntu-latest cargo-machete unused dependency check
typos ubuntu-latest Spell check

Test plan

  • Verify all 6 jobs run in parallel on a PR
  • Confirm fmt/clippy/typos fail fast without waiting for build
  • Validate concurrency cancellation still works on updated pushes

Bisht13 added 3 commits March 25, 2026 13:21
Split the single build_and_test job into separate parallel jobs for
fmt, clippy, build+test, and documentation, matching the pattern from
upstream whir. This gives faster feedback on failures since lightweight
checks (fmt, typos, unused deps) no longer wait behind a full build.
Use --with-metadata flag so machete uses cargo metadata (workspace-aware)
instead of filesystem scanning. Fixes false-pass where machete errored on
playground/cm31_ntt (not a workspace member) but still exited 0.
- bn254-multiplier: ignore hla and bn254-multiplier-codegen (build.rs deps)
- hla: ignore quickcheck (required by quickcheck_macros at expansion time)
@Bisht13 Bisht13 requested review from ashpect and veljkovranic March 25, 2026 11:12
@Bisht13
Copy link
Copy Markdown
Collaborator Author

Bisht13 commented Mar 25, 2026

/claude-review

@github-actions
Copy link
Copy Markdown

Code Review

Summary

This PR refactors the monolithic build_and_test CI job into six focused parallel jobs (build_and_test, cargo_clippy, cargo_fmt, cargo_doc, unused_deps, typos). It also adds --with-metadata to cargo-machete and [package.metadata.cargo-machete] ignore sections to suppress false positives for build-script and proc-macro dependencies.

Review

Correctness & Soundness
No production code changes — this is CI infrastructure only. No soundness concerns.

A few correctness checks:

  • RUSTDOCFLAGS: "--cfg doc_cfg -D warnings" is correctly preserved in the new cargo_doc job. ✓
  • components: rustfmt / components: clippy are properly scoped to only the jobs that need them. ✓
  • Workflow-level concurrency group (${{ github.workflow }}-${{ github.ref }}) still governs all jobs, so push-cancellation continues to work correctly. ✓
  • --with-metadata on cargo-machete is an additive improvement — enables dep graph analysis so build-script and macro deps are better detected before the manual ignores even fire.

Code Quality

  • The three [package.metadata.cargo-machete] ignore entries each carry a clear comment explaining why the dep is suppressed (build.rs, proc-macro expansion). Good.
  • Job names are consistent and descriptive.
  • Minor nit (non-blocking): cargo_fmt runs on ubuntu-24.04-arm but rustfmt output is entirely platform-independent. Switching it (and potentially cargo_clippy/cargo_doc) to ubuntu-latest would free up the ARM slot for the heavier build_and_test job and potentially speed up queue time. That said, using a consistent runner tier is also a defensible choice.
  • Minor nit (non-blocking): cargo clippy does not pass -- -D warnings, so clippy warnings won't fail CI. This matches the previous behaviour and the CLAUDE.md command reference, so it's not a regression — but it's worth noting if stricter lint enforcement is desired in the future.

Testing
CI-only change. The PR description confirms all six jobs ran in parallel and fmt/clippy/typos failed fast. No Rust logic to test.

Suggestions

  • (Non-blocking) Consider moving cargo_fmt to ubuntu-latest to reduce ARM runner load.
  • (Non-blocking) If strict linting is desired, add -- -D warnings to the clippy invocation in cargo_clippy.

Verdict

✅ Approve — clean, well-motivated CI improvement with no regressions.

@Bisht13 Bisht13 merged commit 440631e into main Mar 25, 2026
9 checks passed
@Bisht13 Bisht13 deleted the px/split-ci-parallel-jobs branch March 25, 2026 11:23
dcbuild3r pushed a commit that referenced this pull request May 16, 2026
ci: split monolithic CI into parallel jobs
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.

2 participants