Repository navigation
Keep the release dry-run switch out of the preflight checks - #227
Conversation
`make release VERSION=... DRY_RUN=1` passes DRY_RUN=1 down to the preflight, which runs test-sync-skills.sh. sync-skills.sh reads a variable of the same name and accepts only 'local' or 'remote', so every case failed with "DRY_RUN must be unset, 'local' or 'remote', not '1'" and the dry run could not complete. The test now unsets an inherited DRY_RUN, as it does SKILLS_TOKEN. Each case that wants one sets its own.
Fix the dry run where the sibling CLIs do: release.sh runs release-check without its own DRY_RUN in the environment, and gives the nested make an empty one, since `make release DRY_RUN=1` also carries it through MAKEFLAGS. test-sync-skills.sh goes back to the shared copy unchanged.
There was a problem hiding this comment.
🟢 Approved
No active review issues are recorded for the change isolating skills-sync tests from inherited release dry-run settings.
No open issues 🎉
What changed in this PR
This PR prevents the skills-sync test from failing during the documented make release VERSION=vX.Y.Z DRY_RUN=1 rehearsal.
Changes:
- Clears inherited
DRY_RUNbefore running test cases, leaving individual cases free to set their own value. - Adds a comment explaining the conflict between release and skills-sync dry-run values.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
scripts/test-sync-skills.sh |
Unsets inherited DRY_RUN to isolate test cases from release settings. |
Review effort: Balanced
There was a problem hiding this comment.
No issues found across 1 file
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
What
scripts/release.shrunsmake release-checkwithout its ownDRY_RUNin the environment, and gives the nested make an empty one.Why
make release VERSION=vX.Y.Z DRY_RUN=1, the rehearsal RELEASING.md documents, passedDRY_RUN=1down tomake release-check, which runstest-sync-skills.sync-skills.shreads a variable of the same name and accepts onlylocalorremote, so every case failed:Unsetting the shell variable alone is not enough: a
DRY_RUN=1given on make's command line also travels to the nested make throughMAKEFLAGS. A real release sets noDRY_RUNand was unaffected. basecamp-cli and hey-cli already keep the switch out of the checks in their release scripts; this does the same here.Testing
make checkpassesDRY_RUN=1, a nestedmake test-sync-skillsfails 33 assertions;env -u DRY_RUN make DRY_RUN= test-sync-skillspasses them all.

Confidence Score: 5/5 
The change appears safe to merge.Summary
Clears inherited
DRY_RUNinscripts/test-sync-skills.shso release rehearsals do not pass an unsupported value to the sync script.Reviews (1) · Last reviewed commit: "Let the release dry run pass the skills-..." · Reviewed by Greptile