ReviewOS

also looking at this

craft-native/craft

Stop re-running the first-party Zig dependency setup

#324
Merged glennmichael123 wants to merge fix/duplicate-zig-deps-step into main
3 files +16 -9
9 days ago

Closes #323.

The issue reported one duplicate. Writing a rule instead of deleting the line found four.

WorkflowJobInvocations
ci.ymlzig-core2 — #316 and #317 landed back to back and left the step either side of the native-startup check
release.ymlpantry3 — before the native build, then again before each cross-compile

All are no-ops: the action returns early when a checkout already sits at the SHA in pins.env, and nothing between the steps clears the workspace. In ci.yml that is both matrix legs of every pull request and push.

Checked before deleting: every copy is unconditional. Had the later ones been guarded per-OS — the cross-compile steps beside them are if: runner.os == ... — one would have been load-bearing on the Linux leg and removing it would have broken the release build. They are not, so the first invocation covers every leg.

The rule

A composite action under .github/actions is setup. It clones pinned dependencies or installs a toolchain, and returns early when that work is already done, so a job running one twice is waste rather than intent — and it happens by accident when two pull requests insert steps into the same region, which is exactly how both of these arose. Published actions can legitimately repeat, uploading two artifacts for instance, so only local ones are checked.

Fail-first: the rule fails on ci.yml as it stands, naming the job and the repeated action. On its first run it also surfaced the release.yml trio, which the issue had not noticed. It covers ci.yml, release.yml, sbom.yml and mobile-e2e.yml, so the next accidental copy fails in a pull request rather than surviving in a workflow nobody re-reads.

bunx --bun pickier . 30 warnings / 0 errors (unchanged), bun run verify:docs valid, 6 pass / 0 fail in scripts/release-workflow.test.ts.

Note

release.yml's steps only execute on a tag, so CI here proves the YAML parses and the rule holds, not that the release path still builds. The change removes steps rather than adding any, and the remaining invocation precedes every build step that needs it.

3 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.