Update actions to use versions that use Node v24#3451
Update actions to use versions that use Node v24#3451kevinb-khan wants to merge 3 commits intomainfrom
Conversation
🗄️ Schema Change: No Changes ✅ |
|
Size Change: 0 B Total Size: 495 kB ℹ️ View Unchanged
|
🛠️ Item Splitting: No Changes ✅ |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (f43015f) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3451If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3451If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3451 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
jeremywiebe
left a comment
jeremywiebe
left a comment
There was a problem hiding this comment.
Actual workflow changes are great.
With regards to the script, I have several comments and one more, in general. Will this be something we want to bake in to wonder-stuff as a pnpm dlx-able tool like we did for wonder-stuff-tool-new-pkg?
There was a problem hiding this comment.
Could we upgrade this to be a .ts script using swc? We do that in other scripts in this dir and it at least gives us some type safety.
|
|
||
| // Collect unique action+ref pairs across all files | ||
|
|
||
| const seen = new Map(); // key: "action@ref" → resolved SHA (filled later) |
There was a problem hiding this comment.
Although not entirely necessary, I find moving the contents of the script body into a main function useful so that it's easy to scan the script to see what all does. This script also feels like it could use some functions extracted for the steps it takes.
| run_install: false | ||
|
|
||
| - name: Use Node.js ${{ inputs.node-version }} | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 |
There was a problem hiding this comment.
Was this missed or does it not need to be bumped? The latest version is 6.3.0.
| * Scan all workflow and action YAML files for GitHub Action references and | ||
| * ensure they are pinned to commit SHAs. Handles two cases: | ||
| * 1. Already pinned (`uses: owner/repo@<sha> # <tag>`) — updates stale SHAs | ||
| * 2. Unpinned (`uses: owner/repo@<tag>`) — replaces with `@<sha> # <tag>` |
There was a problem hiding this comment.
When you say "stale", does this mean "not latest" or something else? ie. Is this an upgrade script or just one that transforms our workflow yaml file to all have SHA-pinned actions references?
|
|
||
| const allRepos = new Set(); // all unique owner/repo names (for listing) | ||
|
|
||
| for (const file of files) { |
There was a problem hiding this comment.
These loops all feel like they'd be handy to have in functions so the steps of this script are easier to digest.
| .sort(); | ||
| console.log("Allowed actions:\n"); | ||
| for (const repo of uniqueRepos) { | ||
| console.log(`${repo}@*,`); |
There was a problem hiding this comment.
nit: Can we indent this repo listing and make it a "bulleted" list?
| console.log(`${repo}@*,`); | |
| console.log(` - ${repo}@*,`); |
There was a problem hiding this comment.
Alternately, we could use console.group() and console.groupEnd(). 🤷♂️
| .sort(); | ||
| console.log("Allowed actions:\n"); | ||
| for (const repo of uniqueRepos) { | ||
| console.log(`${repo}@*,`); |
There was a problem hiding this comment.
Also, since the workflow yml files reference actions as repo/name, would we be more clear with this listing if we used:
console.log(`${repo}/*`);
| encoding: "utf-8", | ||
| }).trim(); | ||
|
|
||
| if (branchOutput) { |
There was a problem hiding this comment.
I think if we fall back to a branch ref, the might want to warn. I guess we will still have the changes in the repo to review through normal git workflows... but it feels worth warning about.
2 participants
Summary:
GitHub Actions will no longer be supporting the use of Node v20 in workflow and actions on April 24. That means we need to make sure all workflows and actions are using Node v24 before then.
This PR also includes a script which we use in some of our other repos to keep the SHAs for actions up to date. It updated the SHA based on the version in the comment that comes after it.
Issue: FEI-7601
Test plan: