ci: align build-wasm node setup to setup-node@v6 / Node 24#41243
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Aligns the build-wasm job’s Node.js setup in .github/workflows/cgo.yml with the rest of the repository’s CI configuration by moving off actions/setup-node@v4/Node 20 and standardizing on actions/setup-node@v6/Node 24.
Changes:
- Update
build-wasmto useactions/setup-node@395ad326...(v6). - Bump
node-versionfrom20to24for the wasm golden tests run via Node.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/cgo.yml |
Updates the build-wasm job’s Node.js setup to setup-node@v6 and Node 24 to match the rest of CI. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #41243 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold). Only 1 file changed. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Only .github/workflows/cgo.yml was changed (CI workflow alignment). Test Quality Sentinel skipped. |
|
@copilot refresh the branch and rerun checks after syncing with base.
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out — change looks good and correctly aligns the last outlier.
📋 Key Themes & Highlights
Key Themes
- Consistency alignment:
build-wasmwas the only job still onsetup-node@v4/ Node 20. This PR eliminates that last discrepancy. - SHA verified:
395ad3262231945c25e8478fd5baf05154b1d79fis already pinned inci.yml(line 428) and across allcjs.ymljobs — the new value is a copy-paste from live, working CI, not a freshly-introduced dependency. - Wasm script compatibility:
scripts/test-wasm-golden.mjsuses only stable built-in Node.js APIs (fs,path,child_process,module) — no packages that could break on Node 24.
Positive Highlights
- ✅ Minimal, surgical change — exactly 2 lines, 1 file
- ✅ SHA pin is consistent with the rest of the repo (no version drift)
- ✅ Clear PR description explaining the motivation
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 30.4 AIC · ⌖ 7.47 AIC · ⊞ 6.5K
There was a problem hiding this comment.
No blocking issues — clean alignment change
2-line CI housekeeping: bumps the build-wasm job's Node.js step from setup-node@v4/Node 20 to setup-node@v6/Node 24, eliminating the last outlier in the repo.
Review notes
- SHA correctness:
395ad3262231945c25e8478fd5baf05154b1d79fis identical to every othersetup-nodereference acrossci.yml,cjs.yml, and the other step incgo.yml. - Node 24 compatibility:
test-wasm-golden.mjsruns a WASM binary via Node — Node 24 is fully backward-compatible here and already validated in all other jobs. - No cache needed: This step skips
cache: npmdeliberately — there is nonpm installpreceding it. - Minor (non-blocking):
node-version: '24'uses single-quotes while the other step in the same file uses double-quotes.
🔎 Code quality review by PR Code Quality Reviewer · 42.6 AIC · ⌖ 6.99 AIC · ⊞ 5.2K
The
build-wasmjob incgo.ymlwas the last outlier usingactions/setup-node@v4with Node 20, while every other Node.js step acrossci.ymlandcjs.ymlalready usessetup-node@v6with Node 24.Changes
cgo.yml: Update thebuild-wasm"Set up Node.js" step fromsetup-node@cdca736(v4) +node 20→setup-node@395ad32(v6) +node 24