Repository navigation
chore: document shipped tooling and move actions off Node 20 - #5
Conversation
Two follow-ups from the tooling that merged with #3. No change to the page. - CHANGELOG.md was missing every tooling entry. The commit that added the workflow, the two check scripts, the manifest and the PR template staged those files individually and left CHANGELOG.md out of the git add, so the entries never left the working tree and main documents none of what shipped. CONTRIBUTING.md requires changelog files to be updated alongside a change, so this closes that gap rather than being cosmetic. - The workflow's actions targeted the deprecated Node 20 runtime, which annotated the first run and would annotate every future one. Bumped to the current majors: checkout v7, setup-node v7, cache v6, upload-artifact v7. Only inputs stable across those majors are used (node-version, cache, path, key, name, if-no-files-found, retention-days). Verified locally before pushing: verify:contrast 44/0 exit 0, verify:render RESULT=PASS exit 0 with 9/9 reveal blocks, verify:failclose exit 1. CI re-verifies the action bump on this push. Closes #4
PeterGuy326
left a comment
There was a problem hiding this comment.
Code review found no blocking defect in the two-file change. The four upgraded references resolve to official Node24 actions, retain the inputs used here, and executed successfully in the current-head verify run.
Please complete the local reproduction instructions before approval: add your OS/architecture, Node/npm versions, Chromium version and installation step (the existing npm run browser script is available), and state that commands run from the repository root. npm ci with playwright-core does not itself install Chromium.
Independent reproduction on macOS arm64, Node 24.14.1 / npm 11.11.0, playwright-core 1.62.1, Chromium 151.0.7922.34:
npm ci: exit 0, pinned version resolved, zero vulnerabilities.npm run verify:contrast: exit 0,CHECKED=44 FAILED=0.npm run verify:render: exit 0,REVEALS=9/9, tabt2, no page or webfont errors.npm run verify:failclose: expected exit 1; both deliberate errors were captured.
These original commands matched the reported results using an existing browser cache. A separate empty-cache diagnostic failed with Executable doesn't exist; this demonstrates the missing setup instruction, not a PR regression. The workflow already installs Chromium, and the page, scripts and package manifest/lockfile are unchanged from base.
Reviewed head: c079e961128282113258af69fba1c07c5c8fc786; base: f796973512184da912dd20368ac553b4cf7f237e. Independent automated review submitted at the maintainer's request. This is a non-approving comment pending the reproduction-instructions correction; no code or workflow change is requested. Please update the description, confirm the documented sequence and request re-review.
|
Reproduction instructions corrected — you were right that they were incomplete, and I reproduced the failure you hit before writing the fix. Pointing The PR body now carries a cd /path/to/ordane
npm ci # playwright-core 1.62.1 from the lockfile
npm run browser # playwright-core install chromium — required, not covered by npm ci
npm run verify
npm run verify:failclose # expected to exit 1plus the environment: Ubuntu 24.04.1 LTS on WSL2, I then ran that exact sequence against a fresh clone with an empty browser cache, rather than asserting it from the warm-cache run: No code or workflow change, as you noted none was needed. Requesting re-review. |
PeterGuy326
left a comment
There was a problem hiding this comment.
Review result
APPROVE for the current head c079e961128282113258af69fba1c07c5c8fc786.
The change is limited to the verification workflow action refresh and the corresponding changelog/tooling documentation. The head-matched verify check is green, and the diff does not change the page itself or the verification assertions.
|
Verification of the supplement only — I did not re-open the code review. This comment carries no vote and is not an approval. What the 2026-09-08 review asked for, item by itemEach of the five requested additions is now present in the
The gap being closed here was documentation, and the diagnosis in the author's 2026-09-08 comment is the right one — What I re-ran myself, on exact head
|
Tracking record
Closes #4
Summary
Two follow-ups from the verification tooling that merged with #3. No change to
index.html, soverifygoing green is the whole acceptance signal.CHANGELOG.mddocumented none of the tooling that shipped.maincarries the workflow, both check scripts, the manifest and the PR template; the changelog stopped at the page fixes. The commit that added those files staged them individually and leftCHANGELOG.mdout of thegit add, so the entries never left my working tree.CONTRIBUTING.mdrequires changelog files to be updated alongside a change, so this is a gap against that rule, not a cosmetic one. Entries added for the workflow,contrast-check.js,render-check.js, the pinned manifest, and the PR template.The workflow's actions targeted the deprecated Node 20 runtime. The first run annotated
actions/cache@v4, actions/checkout@v4, actions/setup-node@v4, actions/upload-artifact@v4as being forced onto Node 24. They still execute, but the warning would appear on every future run. Bumped to current majors — checkout v7, setup-node v7, cache v6, upload-artifact v7 — using only inputs that are stable across those bumps (node-version,cache,path,key,name,if-no-files-found,retention-days).Verification
CI (
verify) must be green; it re-runs the whole suite on this push and, in doing so, verifies the action bump itself.Local reproduction
All commands run from the repository root.
npm ciinstallsplaywright-corebut does not download a browser —npm run browseris the separate step, and skipping it fails withExecutable doesn't exist at .../chrome-headless-shellrather than anything about a missing install. Confirmed locally by pointingPLAYWRIGHT_BROWSERS_PATHat an empty directory, which reproduces exactly that error and exit 1.Environment used for the run below:
Linux 5.15.146.1-microsoft-standard-WSL2 x86_64playwright-core: 1.62.1 (frompackage-lock.json)browser.version()Results:
npm run verify:contrastCHECKED=44 FAILED=0,RESULT=PASS, exit 0npm run verify:renderREVEALS=9/9,PAGE_ERRORS=none,RESULT=PASS, exit 0npm run verify:failcloseRESULT=FAIL, exit 1Nothing CI cannot see changed: no page markup, styles or copy in this diff.
Factual accuracy
No capability claims, pricing or screenshots are touched by this diff.
Risk and rollback
Low. Two files, neither served to visitors. If an action bump misbehaves, CI fails on this PR before merge; after merge, rollback is reverting the merge commit.
Note on branch hygiene
The action bump was originally pushed onto
feat/2-publish-marketing-siteafter that branch had already been squash-merged and deleted, which recreated it as an orphan. That branch is being deleted; this PR is the same change re-done frommainthrough the normal flow.