Repository navigation
fix(pr-validation): say which half of the gate failed, not just that it did - #946
Merged
Merged
Conversation
…it did The blocked comment said only that the gate did not reach a verdict and linked the run. True, and no help: the author opens the run and hunts for the red job. Measured on ungoliant-controller, whose first breaking change produced exactly that comment. The cause was three steps away in the log - no catch-all CODEOWNERS rule, no fallback reviewer, so the Environment could not be created - and nothing on the pull request pointed at any of it. The verdict already knew which half failed; it was writing it to the log and telling nobody else. It now emits the stage, the outcome forwards it, and the comment names it. The request case also names the cause that actually occurs, since it is the one a reader cannot guess. Empty keeps the neutral wording, so a caller that does not pass it is no worse off. @radagast-lerian offered both options on #926 and I took the cheaper one; this is the other.
|
Warning Review limit reachedThe included review limit has been reached and this organization has disabled usage-based review continuation. Wait for reviews to reset or ask a billing admin to change After included review limits.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Next included review available in 12 minutes. View limit details
Comment |
lerian-studio
added
size/S
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
Contributor
🔍 Lint Analysis
|
Contributor
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
Contributor
|
@coderabbitai review |
lerian-studio
added
the
review-ready
|
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.
Description
Found by running the gate for real.
LerianStudio/ungoliant-controller#395— the first breaking change in a repository that consumesgo-pr-validation@tier-0— produced this comment and nothing else:True, and no help. The actual cause was three steps away in the job log:
Nothing on the pull request pointed at it. The author has to open the run, find the red job among the green ones, and read the log.
The verdict already knew. It resolves
DETECT_RESULT,REQUEST_RESULTandDETECTION_SUCCEEDED, and writes them to an::error::— to the log, and to nobody else.It now emits the stage, the outcome forwards it as
blocked-stage, and the comment names it:detection— detection did not complete, so the gate does not know whether there is a break at allrequest— the break was detected but the approval could not be requested. This one also names the cause that actually happens, since a reader cannot guess it: the Environment does not exist and could not be created, because either no reviewer could be resolved (no catch-all CODEOWNERS rule, no fallback configured) or the token cannot create EnvironmentsEmpty keeps the previous neutral wording, so a caller that does not pass it is no worse off.
@radagast-lerian offered exactly these two options on #926 — "Use neutral gate-failure wording or forward the failed stage". I took the cheaper one. This is the other, and the run above is why it was the right one.
Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBreaking Changes
None.
blocked-stageis a new optional input on the reporter composite, defaulting to empty, and empty renders exactly the current text.Testing
@this-branchor the beta tagactionlintclean,shellcheckclean on the reporter,typosclean. 115 workflow tests, 60 detector, 32 gate-scope, 28 commit-list, 22 CODEOWNERS.test_a_blocked_gate_names_the_half_that_failedrenders all three cases, checks each excludes the other's wording, and pins that the verdict resolvesdetectionandrequestfrom the right inputs and leaves the stage empty when nothing is blocked. Verified by mutation: deleting the stage resolution fails it.Caller repo / workflow run: https://github.com/LerianStudio/ungoliant-controller/actions/runs/39441483943 — the run that produced the unhelpful comment.
Related Issues
Follows the
blockedstate added in #935 and the option @radagast-lerian raised on #926.