Source: merge commit bd4b7e6826a7e6056cd1ca5514e7ffa4f1cc1efa — review-heavy (3 substantive review rounds).
Proposed learning: derive required-status-check context strings from the workflow instead of restating them. The reviewer's central observation was that the interesting part of the change was not the one-line settings edit promoting a job to a required check — it was that the change stopped two files from agreeing by handshake. The accompanying test reads the context name out of the workflow's own job definition (jobs.<job>.name) and asserts membership in the required-checks list, rather than hardcoding the same literal a second time. With that shape, the two sides can only drift if someone deliberately renames both. Any byte-for-byte contract between a workflow file and a branch-protection settings file should be expressed as a derived assertion, not as duplicated string literals.
Second half of the learning: the pre-flight checklist for promoting any job to a required check. A required context that can be skipped deadlocks the branch, so the reviewer verified the claim by reading the trigger surface rather than taking it on faith — no paths: filter on the workflow, pull_request.types covering the full opened/reopened/synchronize/ready-for-review set, a job-level if: that is unconditional for pull requests, and sibling jobs already riding the same triggers and checkout ref. The load-bearing detail is where the applicability gate lives: the changed-file check runs inside the script, short-circuiting to a not-applicable result with exit 0, so the context reports on every PR. An equivalent condition expressed as a job-level if: would have produced a permanently pending check and a merge deadlock.
Suggested capture: docs/solutions/workflow-issues/, cross-referencing the existing note on quoting required status check contexts (the review confirmed the unquoted form is correct here because the name carries no YAML-significant punctuation). Scope it to two reusable pieces: the test pattern for workflow-to-settings contracts, and the can-this-context-ever-fail-to-report checklist covering triggers, path filters, job-level conditions, and gate placement.
Source: merge commit
bd4b7e6826a7e6056cd1ca5514e7ffa4f1cc1efa— review-heavy (3 substantive review rounds).Proposed learning: derive required-status-check context strings from the workflow instead of restating them. The reviewer's central observation was that the interesting part of the change was not the one-line settings edit promoting a job to a required check — it was that the change stopped two files from agreeing by handshake. The accompanying test reads the context name out of the workflow's own job definition (
jobs.<job>.name) and asserts membership in the required-checks list, rather than hardcoding the same literal a second time. With that shape, the two sides can only drift if someone deliberately renames both. Any byte-for-byte contract between a workflow file and a branch-protection settings file should be expressed as a derived assertion, not as duplicated string literals.Second half of the learning: the pre-flight checklist for promoting any job to a required check. A required context that can be skipped deadlocks the branch, so the reviewer verified the claim by reading the trigger surface rather than taking it on faith — no
paths:filter on the workflow,pull_request.typescovering the full opened/reopened/synchronize/ready-for-review set, a job-levelif:that is unconditional for pull requests, and sibling jobs already riding the same triggers and checkout ref. The load-bearing detail is where the applicability gate lives: the changed-file check runs inside the script, short-circuiting to a not-applicable result with exit 0, so the context reports on every PR. An equivalent condition expressed as a job-levelif:would have produced a permanently pending check and a merge deadlock.Suggested capture:
docs/solutions/workflow-issues/, cross-referencing the existing note on quoting required status check contexts (the review confirmed the unquoted form is correct here because the name carries no YAML-significant punctuation). Scope it to two reusable pieces: the test pattern for workflow-to-settings contracts, and the can-this-context-ever-fail-to-report checklist covering triggers, path filters, job-level conditions, and gate placement.