-
Notifications
You must be signed in to change notification settings - Fork 296
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
Bump pnpm
to v8, node
to v20
#1185
Conversation
|
…ct into pnpm-node-bump
"pnpm": { | ||
"patchedDependencies": { | ||
"[email protected]": "patches/[email protected]" | ||
} | ||
}, | ||
"volta": { | ||
"node": "20.9.0" |
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.
Can we keep the pnpm
volta config, or does this cause an error if you don't enable have the volta envar set?
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.
We can, however it's prone to be forgotten (as was the case with this PR). Upgrading pnpm means you need to change packageManager
, as well as the volta.pnpm
key.
packageManager
is more of a de facto standard, which is why I added it.
Doing a bit of spring cleaning 🧹. Up first is
pnpm
andnode
. Nothing of note fornode
, but the newpnpm
major came with a new lockfile format.NOTE: Not dropping support for any specific version of node in this PR. We don't even officially have a supported version anyway (AFAIK), but I think it's safe to drop testing on 14 given it became EOL ~4 months ago. 16 just became EOL a few days ago, but happy to keep it around for now.
This might also require a tweak to the branch protection since it's expecting a
node 14
CI step to pass but it's now namednode 16
.