Make the merge the release, and every check the thing that stands in front of it - #78
Merged
Merged
Conversation
…front of it The gates were at the wrong end. A pull request landed on one green check while Coverage, Playwright and Visual check only reported, and then the release was held a second time by a reviewer on the production environment — so merging proposed a deploy that somebody still had to approve. Move the weight forward. `main`'s ruleset now requires all four checks and that the branch is up to date with main, and fly-deploy.yml deploys the merge commit with no gate job and nothing to approve. The two are a pair: nothing re-runs on main any more, so what the pull request proved is the last word on what reaches the volume, and the branch cannot have gone stale underneath it. Requiring Coverage is safe because its floor is on the lines a change touched — a pull request with no coverable line in it measures nothing and passes rather than wedging. Requiring the browser jobs costs a flake blocking a merge; the answer is re-running the job, since bypass_actors is still empty. ci.yml loses the workflow_call trigger and the three guards that opted the advisory jobs out of it, having no caller left. The production environment stays for its secret and its branch policy, which is what keeps FLY_API_TOKEN out of reach of a workflow on a feature branch. Only its reviewer box is emptied, and deployment.md says how to tick it again for a release worth sitting on. Also written down, since a merge is now a release: the five routes by which another account could ever come to press that button, all five of which start with an invite, an app, or a key added here — and the second ruleset to add on the day one of them is. Two settings have to be applied by hand, as ever: re-import the ruleset under its new name, and untick Required reviewers on the production environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsfcX5Qc2aFq59gVYtmnB4
Two review passes over the previous commit found the workflows and the ruleset sound and the documentation half a design behind. Four files still described the gate that commit deleted, and three claims inside the prose it rewrote argued with themselves. Retire the gate properly. testing.md led with the deploy job depending on the test run, and then offered a push deploying straight to production as the bad old days — which is the design again, so it now says what actually stands between a merge and the volume: the Docker build and the smoke check. The three one-line versions of the same claim, in CLAUDE.md, README.md and project_overview.md, become the tests gating the merge, which is what they do. Then the contradictions. `pull_request` is not the only event reporting these checks — workflow_dispatch reports the same four contexts and can unlock a merge on its own, off the branch tip rather than the merge ref, which matters because the paragraph making the claim then recommends it as the escape hatch. "Two things could put a release on the volume" was followed by three of them; the approval that went was a mitigation, not a route. And the Actions-token row of the new access table named the wrong prerequisite and the wrong backstop: it takes contents: write, not the create-and-approve setting, a workflow on a branch never has to be merged to run, and what actually holds is the ruleset binding GITHUB_TOKEN like anybody else. Said out loud too that disabling the ruleset costs more than it did. It used to skip one check and leave the commit built and tested on main regardless; now it merges, deploys and migrates an unbuilt commit. The one behaviour change is on the manual route, which lost the gate job and the reviewer in the same commit and kept nothing. A dispatch now takes a confirm input that has to read "deploy". The push arm of the condition is untouched and tests the ref alone, so a merged pull request still releases with nothing to fill in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsfcX5Qc2aFq59gVYtmnB4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gates were at the wrong end. A pull request landed on one green check while
Coverage, Playwright and Visual check only reported, and then the release was
held a second time by a reviewer on the production environment — so merging
proposed a deploy that somebody still had to approve.
Move the weight forward.
main's ruleset now requires all four checks and thatthe branch is up to date with main, and fly-deploy.yml deploys the merge commit
with no gate job and nothing to approve. The two are a pair: nothing re-runs on
main any more, so what the pull request proved is the last word on what reaches
the volume, and the branch cannot have gone stale underneath it.
Requiring Coverage is safe because its floor is on the lines a change touched —
a pull request with no coverable line in it measures nothing and passes rather
than wedging. Requiring the browser jobs costs a flake blocking a merge; the
answer is re-running the job, since bypass_actors is still empty.
ci.yml loses the workflow_call trigger and the three guards that opted the
advisory jobs out of it, having no caller left.
The production environment stays for its secret and its branch policy, which is
what keeps FLY_API_TOKEN out of reach of a workflow on a feature branch. Only
its reviewer box is emptied, and deployment.md says how to tick it again for a
release worth sitting on.
Also written down, since a merge is now a release: the five routes by which
another account could ever come to press that button, all five of which start
with an invite, an app, or a key added here — and the second ruleset to add on
the day one of them is.
Two settings have to be applied by hand, as ever: re-import the ruleset under
its new name, and untick Required reviewers on the production environment.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01XsfcX5Qc2aFq59gVYtmnB4