-
Notifications
You must be signed in to change notification settings - Fork 1.2k
ci: gate the docs site build on pull requests #4895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+127
−3
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
58 changes: 58 additions & 0 deletions
58
...n/260917_l1_preview_read_fence_and_dep_audit/030_unit_c_docs_site_build_gate.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # Unit C — a pull-request build gate for the docs site | ||
|
|
||
| Opened because unit B's review could not honestly close. The Astro 7.2 to 7.3 bump | ||
| in #4873 had no pull-request build gate anywhere, so no amount of green CI on that | ||
| head was evidence the docs site still built. The alternative was to accept an | ||
| author's local result as the record, and that is not a standard this repository | ||
| applies elsewhere. | ||
|
|
||
| ## The gap | ||
|
|
||
| `.github/workflows/ci.yml` contained no `docs-site` reference and built no docs. | ||
| `deploy-docs.yml` triggers only on `push` to `main`, which is after promotion. So | ||
| the first machine to discover a broken docs build was the deploy, and the only | ||
| pre-merge evidence available was an unverifiable attestation. | ||
|
|
||
| ## Shape | ||
|
|
||
| A new `docs` filter in the existing `changes` job, and one job selected by it. | ||
|
|
||
| `docs-site/**` is deliberately **not** added to the `ci` filter. Widening `ci` | ||
| would start the whole cross-platform matrix for a prose edit, which is a cost with | ||
| no matching evidence: a docs change has to build, not to pass the runtime suite. | ||
| A separate filter output keeps the two questions apart. | ||
|
|
||
| `.github/workflows/ci.yml` is in the `docs` filter so an edit to the job verifies | ||
| itself. Without that entry this unit's own pull request would skip the job it adds, | ||
| which is the failure mode the unit exists to remove. | ||
|
|
||
| One Linux leg. The site is static output from a Node/Bun toolchain with no | ||
| OS-specific behaviour to promise, so a Windows or macOS leg would spend queue time | ||
| without buying coverage. `--frozen-lockfile` carries as much of the value as the | ||
| build does: it fails on a manifest and lockfile that disagree, which is exactly the | ||
| shape a hand-edited override introduces. | ||
|
|
||
| ## Constraints honoured | ||
|
|
||
| No existing job's sharding, timeout, or runner selection is touched. No new Windows | ||
| leg. Workflow-level `permissions` stay `contents: read` and the job adds none. | ||
| Actions stay pinned to immutable SHAs with their version comments. | ||
|
|
||
| The aggregate gate is the part that is easy to get wrong. `ci` is event-aware since | ||
| #4837: it derives what the event requested and demands `success` from each requested | ||
| job and `skipped` from every other one, and it fails by name on any job with no | ||
| declared expectation. So the job is added in four places that must agree: | ||
| `needs`, `CHANGES_DOCS`, `GATED_JOBS`, and `expected_for`. And | ||
| `tests/ci-workflows/ci-workflows.test.ts` independently derives the expected | ||
| `needs` list from the workflow's own job keys, so a missing entry fails rather than | ||
| passing quietly. | ||
|
|
||
| `structure/ops/docs-and-release.md` owns `docs-site/` and is updated in the same | ||
| change, including the workflow map. | ||
|
|
||
| ## Sequencing | ||
|
|
||
| This unit lands before #4873 merges. #4873 then needs a fresh run for the new job | ||
| to appear on its head; a re-run is the cheap way to find out whether the merge-ref | ||
| workflow already carries it, before considering anything that rewrites that branch. | ||
| Both calls belong to the host. |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR gate installs the root-pinned Bun version (1.4.0 in this commit), but
.github/workflows/deploy-docs.ymlstill builds with Node 22 andpackage-manager: bun@latest. Oncebun@latestdiffers—or if the Node-backed Astro action behaves differently—a docs change can pass this gate and then fail during the post-promotion Pages build, defeating the gate's stated purpose. Pin the deploy workflow to the same toolchain or exercise the deployment action/configuration here.Useful? React with 👍 / 👎.