Skip to content

Move nix CI jobs to separate workflow#708

Open
epompeii wants to merge 1 commit intodevelfrom
u/ep/ci-nix
Open

Move nix CI jobs to separate workflow#708
epompeii wants to merge 1 commit intodevelfrom
u/ep/ci-nix

Conversation

@epompeii
Copy link
Member

The 3 nix jobs (lints, build bencher, build api) are fully independent of the main CI pipeline. Extract them into a standalone nix.yml workflow to simplify lint.yml and build.yml.

@epompeii epompeii self-assigned this Mar 16, 2026
@github-actions
Copy link

github-actions bot commented Mar 16, 2026

🤖 Claude Code Review

PR: #708
Base: devel
Head: u/ep/ci-nix
Commit: f46c9fb7a00bb03a149ecacad7250568f5aeacda


Here's my review of this PR:


PR Review: Move nix CI jobs to separate workflow

Summary

This PR extracts Nix-related CI jobs (lint checks, bencher Nix build, api Nix build) from build.yml and lint.yml into a new standalone nix.yml workflow, and adds a changes job with path-based filtering so Nix jobs only run when relevant files change.

Positives

  • Good use of path filtering — the changes job with dorny/paths-filter avoids unnecessary Nix builds, saving CI time. The filter patterns for cli and api correctly mirror those in ci.yml.
  • Consistent structure — the new workflow follows the same patterns as the rest of the CI (same runner, same Nix install action, same nixpkgs channel).
  • CLAUDE.md update — the Co-Authored-By instruction is a useful addition for agent workflows.

Issues

1. Nix workflow is decoupled from CI gate (Medium)

The ci-success job in ci.yml (lines 168-204) gates merges by checking all required jobs. The new nix.yml is a completely separate workflow and is not checked by ci-success. If the Nix build breaks, PRs can still merge without noticing. If this is intentional (Nix builds are advisory), that's fine — but if Nix builds should be required, you'll need to either add nix.yml as a required status check in branch protection rules or keep it within the CI workflow.

2. Missing cloud branch skip on changes job vs. other jobs (Low)

In ci.yml, the changes job runs unconditionally but downstream jobs check github.ref != 'refs/heads/cloud'. In nix.yml, the changes job itself has if: github.ref != 'refs/heads/cloud', which means the downstream jobs that need: changes will be skipped on cloud pushes. This works correctly but differs from the pattern in ci.yml. Minor inconsistency, not a bug.

3. Path filter for cli is missing services/api/openapi.json (Low)

In ci.yml:61, the cli filter includes services/api/openapi.json because the CLI client is generated from the OpenAPI spec. The nix.yml cli filter omits this. If the Nix bencher package depends on the generated client, a spec-only change wouldn't trigger a rebuild. This likely doesn't matter since the Nix build would pull from Cargo.lock-resolved deps, but worth noting for correctness.

4. No workflow_dispatch inputs (Nit)

The workflow_dispatch trigger has no inputs, which is fine for now but means you can't manually trigger for a specific ref with a deploy-only flag like build.yml supports. Not blocking.

Security

No concerns — no secrets are used, no new permissions granted, and the workflow runs on the same ubuntu-22.04 runners.

Verdict

This is a clean, straightforward extraction. The main question is whether Nix builds should remain a merge gate (issue #1 above). If they're intentionally advisory, this looks good to merge.


Model: claude-opus-4-6

The 3 nix jobs (lints, build bencher, build api) are fully independent
of the main CI pipeline. Extract them into a standalone nix.yml workflow
to simplify lint.yml and build.yml.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant