Skip to content

ci: only run CUDA lint when CUDA files change and use 8-cpu runner#2563

Merged
jonathanpwang merged 2 commits into
develop-v2.0.0-betafrom
fix/cuda-lint-trigger
Mar 16, 2026
Merged

ci: only run CUDA lint when CUDA files change and use 8-cpu runner#2563
jonathanpwang merged 2 commits into
develop-v2.0.0-betafrom
fix/cuda-lint-trigger

Conversation

@jonathanpwang
Copy link
Copy Markdown
Contributor

Summary

  • Add a changes detection job using dorny/paths-filter@v3 to skip the lint-cuda job when no CUDA-related files (*.cu, *.cuh, **/cuda/**, **/cuda*.rs, etc.) are modified
  • Request at least 8 CPUs on the GPU runner (/cpu=8) for faster builds
  • The workflow file and CUDA cache action are also included as triggers so changes to CI itself still run the CUDA lint

Test plan

  • Open a PR that doesn't touch any CUDA files and verify lint-cuda is skipped
  • Open a PR that touches a .cu or .cuh file and verify lint-cuda runs

Add a `changes` detection job using dorny/paths-filter to skip the
lint-cuda job when no CUDA-related files (*.cu, *.cuh, cuda/**, etc.)
are modified. Also request at least 8 CPUs on the GPU runner.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 16, 2026

Claude finished @jonathanpwang's task in 1m 22s —— View job


Code Review: PR #2563

  • Read changed workflow file
  • Analyze CUDA file patterns in the repo
  • Provide review feedback

Overall this looks good — clean use of dorny/paths-filter and the conditional is wired up correctly. A few observations:

1. Pattern **/cuda*.rs is broader than needed (minor)

The pattern **/cuda*.rs matches files like cuda_abi.rs, cuda_utils.rs, and cuda.rs — which is the intent. But it would also match a hypothetical cuda_unrelated_foo.rs at any depth. This isn't a real problem today since all files matching this pattern are indeed CUDA-related (I checked), just noting it's a greedy glob. The separate **/cuda_abi.rs pattern is actually redundant since **/cuda*.rs already covers it.

2. build.rs files referencing CUDA are not included in triggers

There are several build.rs files (e.g., crates/vm/build.rs, extensions/rv32im/circuit/build.rs) that contain CUDA compilation logic gated behind #[cfg(feature = "cuda")]. Changes to these files could affect the CUDA build but wouldn't trigger lint-cuda. However, build.rs changes are uncommon and would typically accompany changes to *.cu/*.cuh files anyway, so this is low risk.

3. Cargo.toml / feature flag changes not covered

If someone adds or modifies a cuda feature in a Cargo.toml without touching any .cu/.rs files, the CUDA lint would be skipped. Again, low probability since feature changes usually accompany code changes, but worth noting.

4. Consider adding **/build.rs to the filter (optional)

If you want to be more conservative, you could add **/build.rs to the filter list. The cost is just running CUDA lint on the (relatively rare) build.rs-only PRs.

5. The changes job runs on every PR

The changes job runs on ubuntu-latest for every PR even when only lint-cuda uses its output. This is cheap (a few seconds) so not a concern — just confirming it's intentional. The lint job doesn't depend on changes, which is correct.

Verdict

LGTM — the approach is straightforward, the filter patterns cover the important cases, and the 8-CPU runner change is a nice improvement. The edge cases mentioned above are unlikely to cause issues in practice.

Copy link
Copy Markdown
Contributor

@stephenh-axiom-xyz stephenh-axiom-xyz left a comment

Choose a reason for hiding this comment

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

LGTM except I'd add build.rs files

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 16, 2026

Claude finished @jonathanpwang's task in 43s —— View job


Code Review: PR #2563

  • Read changed workflow file
  • Analyze changes against base branch
  • Provide review feedback

The changes look good. Here's my review of the current state (after both commits):

Changes Summary

  1. New changes job using dorny/paths-filter@v3 to detect CUDA-related file modifications
  2. lint-cuda job now conditionally runs only when CUDA files change (needs: changes + if condition)
  3. GPU runner requests 8 CPUs (/cpu=8) for faster builds
  4. Second commit added **/build.rs to the filter patterns per reviewer feedback

Observations

The **/build.rs pattern is quite broad (line 29): This triggers CUDA lint on any build.rs change in the repo, even in crates with no CUDA involvement. A more targeted approach would be to list specific build.rs files that actually contain CUDA logic (e.g., crates/vm/build.rs, extensions/rv32im/circuit/build.rs). However, build.rs changes are infrequent enough that the extra runs are cheap — this is a pragmatic tradeoff and fine to keep as-is.

Redundant pattern: **/cuda_abi.rs (line 28) is already covered by **/cuda*.rs (line 27). Minor, no functional impact.

Workflow structure is correct: The changes job runs on ubuntu-latest (cheap), lint-cuda properly depends on it via needs + if, and the lint job is independent — all wired up correctly.

Verdict

LGTM. The approach is clean and the filter patterns cover the important cases. The two minor nits above are cosmetic and don't need to block merge.


@jonathanpwang jonathanpwang merged commit f7b7c14 into develop-v2.0.0-beta Mar 16, 2026
4 checks passed
@jonathanpwang jonathanpwang deleted the fix/cuda-lint-trigger branch March 16, 2026 19:12
jpw-axiom pushed a commit that referenced this pull request May 16, 2026
…2563)

## Summary
- Add a `changes` detection job using `dorny/paths-filter@v3` to skip
the `lint-cuda` job when no CUDA-related files (`*.cu`, `*.cuh`,
`**/cuda/**`, `**/cuda*.rs`, etc.) are modified
- Request at least 8 CPUs on the GPU runner (`/cpu=8`) for faster builds
- The workflow file and CUDA cache action are also included as triggers
so changes to CI itself still run the CUDA lint

## Test plan
- [ ] Open a PR that doesn't touch any CUDA files and verify `lint-cuda`
is skipped
- [ ] Open a PR that touches a `.cu` or `.cuh` file and verify
`lint-cuda` runs

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
jpw-axiom pushed a commit that referenced this pull request May 18, 2026
…2563)

## Summary
- Add a `changes` detection job using `dorny/paths-filter@v3` to skip
the `lint-cuda` job when no CUDA-related files (`*.cu`, `*.cuh`,
`**/cuda/**`, `**/cuda*.rs`, etc.) are modified
- Request at least 8 CPUs on the GPU runner (`/cpu=8`) for faster builds
- The workflow file and CUDA cache action are also included as triggers
so changes to CI itself still run the CUDA lint

## Test plan
- [ ] Open a PR that doesn't touch any CUDA files and verify `lint-cuda`
is skipped
- [ ] Open a PR that touches a `.cu` or `.cuh` file and verify
`lint-cuda` runs

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

3 participants