Repository navigation
feat(rust-ci): add gate jobs whose names cannot move - #145
Merged
Merged
Conversation
A required status check is matched by its name string, and two jobs here build
their name by interpolating a value: `Test (${{ matrix.os }})` and
`MSRV (${{ inputs.msrv }})`. When a consumer changes that value the job emits a
different name, the consumer's ruleset still requires the old one, nothing emits
it, and the pull request waits forever — and it is the pull request carrying the
change, so the fix cannot be merged from inside it.
`MSRV` is the worse of the two: not a matrix, but a single job interpolating an
input into its own name.
Renaming the existing jobs would be the obvious fix and is the wrong one: it
would break every ruleset that requires the current names, with a failure that
is harder to diagnose than a renamed input, because the input is in a file and
the check name is in a settings page nobody opens.
So this is additive. Two new jobs carry names that cannot move, `Extra platforms`
and `MSRV`, and consumers migrate their rulesets to those at their own pace. The
value-carrying names keep emitting; nothing is removed here.
Both gates use `if: always()` and inspect `needs.<job>.result` rather than
relying on `needs:` alone, because `needs:` alone SKIPS the dependent job when
its dependency fails, and a skipped required check does not block a merge — the
exact failure these jobs exist to prevent. `skipped` counts as a pass, since
both upstream jobs are conditional on caller inputs.
Refs Glyndor/podup#1552
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
The generated page tells a consumer which checks are safe to require, since a required check whose name nothing emits blocks every pull request. Its rule for "conditional" was `job.if is not None`, so the two gate jobs added in the previous commit — which carry `if: always()` precisely so they keep emitting when the job they report on failed or was skipped — were documented as conditional, with `always()` printed in the "Emitted when" column. That inverts the meaning of the table on exactly the rows a ruleset should be pointed at: it warned readers away from the only two checks in this workflow that cannot become phantoms. `rust-ci` now reads 6 of 10 conditional rather than 8 of 10, and both gates read `always`. No other reusable page changes: none of them spells out `always()`. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
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
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.
First step of Glyndor/podup#1552, and the one that has to land before anything else can move.
The problem
A required status check is matched by its name string. Two jobs here build their name by interpolating a value:
When a consumer changes that value, the job emits a different name, the consumer's ruleset still requires the old one, nothing emits it, and the pull request waits forever — and it is the pull request carrying the change, so the fix cannot be merged from inside it.
MSRVis the worse of the two: not a matrix, but a single job interpolating an input into its own name. The number is informative in the UI and poisonous as a gate name, and it is in the job log either way.Why not just rename the jobs
Dropping the interpolation would emit
MSRV, which no existing ruleset requires — blocking every pull request in every consumer at once. A check-name change is at least as breaking as renaming an input and harder to diagnose: the input is in a file, the name is in a settings page nobody opens.So this is additive. Two new jobs carry names that cannot move, and consumers migrate their rulesets at their own pace.
Test (${{ matrix.os }})andMSRV (${{ inputs.msrv }})keep emitting; nothing is removed here.The one detail that makes them gates rather than decorations
needs:on its own skips the dependent job when its dependency fails, and a skipped required check does not block a merge — the exact failure these jobs exist to prevent.if: always()plus an explicitneeds.<job>.resultcheck is what makes them gate.skippedcounts as a pass, because both upstream jobs are conditional on caller inputs (inputs.msrv != '',inputs.extra-test-os != '[]'), so a caller that asks for neither has nothing to fail.The reasoning is in the YAML as comments on purpose. It is what stops someone deleting the
if: always()in six months for looking redundant.Second commit: the generated docs were about to lie
docs/reusables/rust-ci.mdtells a consumer which checks are safe to require. Its rule for "conditional" wasjob.if is not None, so the two new gates — which carryif: always()precisely so they always emit — came out documented as conditional, withalways()printed in the "Emitted when" column.That inverts the table's meaning on exactly the rows a ruleset should be pointed at: it warned readers away from the only two checks in this workflow that cannot become phantoms. The page now reads 6 of 10 conditional instead of 8 of 10. No other reusable page changes — none of them spells out
always().Verification
The gate logic was executed against all four values
needs.<job>.resultcan take, rather than reasoned about:needs.resultsuccessMSRV: successskippedMSRV: skippedfailure::error::MSRV job did not pass (result: failure)cancelled::error::MSRV job did not pass (result: cancelled)actionlint .github/workflows/rust-ci.ymlclean. The docs renderer is idempotent — regenerating into a copy and diffing produces no change.What happens next, and in this order
podman-vmmatrix, and lands thatConsumers other than podup will see two new checks appear on their pull requests. They are not required anywhere, so nothing blocks.