-
Notifications
You must be signed in to change notification settings - Fork 24
chore: create common action to setup Node.js and correct version of npm #2652
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
Conversation
| - name: Set up NodeJs | ||
| uses: actions/setup-node@v6 | ||
| - name: Setup Node and npm | ||
| uses: ./.github/workflows/common/setup-node-and-npm |
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.
I'm seeing the use NODE_AUTH_TOKEN and want to make sure that is different from what Scott mentioned for
And remove any reference to npm config that uses NPM_AUTH_TOKEN.
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.
It's only referenced in the description for the registry-url input of the new custom action. That description was copied from actions/setup-node@v6 because it's just a pass-through. I don't believe we're setting NPM_AUTH_TOKEN anywhere in our CI in this repo anymore. Unless it's configured at the organization level, but that's outside the scope of this PR.
I think registry-url is still relevant to use...
| node-version: | ||
| description: 'Version Spec of the version to use. Examples: 12.x, 10.15.1, >=10.15.0.' | ||
| required: false | ||
| default: '22.x' |
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.
non-blocking: Is 22 the one we still want or should we go up to 24.x?
shannonwells
left a comment
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.
Just the one question but I think this is fine assuming Aramik's Q about NODE_AUTH_TOKEN is resolved.
enddynayn
left a comment
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.
👍 great!
We need to use npm v11.x, but we need to stick with earlier versions of node, which have 10.x. Pull in a version of the solution used with this [frequency-chain PR](frequency-chain/frequency#2652)
Goal
The goal of this PR is to ensure that our CI pipeline is using at least version
>= 11.5.1ofnpmin order to support OID auth from GitHub.It refactors many individual references to
actions/setup-nodeinto a composite action that incorporates bothactions/setup-nodeas well as the specific commands to ensure a recent enough version ofnpm.Another benefit is that the default version of Node.js for all of our CI workflows is now specified in a single place (but can stlll be overridden per invocation if necessary)