-
Notifications
You must be signed in to change notification settings - Fork 19
build: update pnpm #929
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
build: update pnpm #929
Conversation
| strategy: | ||
| matrix: | ||
| node-version: [16.x, 18.x] | ||
| node-version: [18.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.
pnpm 9.0.0 discontinued support for node 16
.node-version
Outdated
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 deleted this file rather than updating it because the node.js version is inherently the one thing we simply cannot guarantee will match at runtime. If a developer updates to a more recent version of node and discovers that something broke, that's useful, and we shouldn't be trying to prevent that.
305d3fc to
eadd3ef
Compare
|
The title originally mentioned |
9acaeaa to
87db329
Compare
87db329 to
dd185e8
Compare
|
Now that all of my commits are verified, I should be done force-pushing. I'll explicitly mention if that changes. |
| - './packages/*' | ||
| - './packages/*' | ||
|
|
||
| autoInstallPeers: false |
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.
This option reverts to the value that used to be the default in pnpm 7. We don't technically need it, but in my view the new default is a downgrade.
| - './packages/*' | ||
|
|
||
| autoInstallPeers: false | ||
| linkWorkspacePackages: deep |
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 believe the default behavior in pnpm 7 might have merely been true, but regardless, deep is very desirable since packages in this repo have dependencies on their siblings that go more than one layer deep.
|
I've gone through and improved the descriptions of all open PRs (I assume you haven't read any of them yet, but I thought I'd mention it just in case). |
eventualbuddha
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.
Thank you for taking on these maintenance tasks! This looks good to me, but I have one quick question.
| - uses: pnpm/action-setup@v2 | ||
| with: | ||
| version: 7 | ||
| - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.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 not use the v4.1.0 tag?
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 could, but a tag can be later updated, so if a malicious person gained write access to the pnpm actions repo, they could run malicious code in this workflow without us having to do anything.
Specifying a commit provides the same protection for actions that checking in a lockfile with hashes does for npm dependencies.
I'm less worried about the official GitHub ones. In all likelihood, it would be very difficult for a rogue GitHub employee to deploy malicious code, and if they did, ~everyone would be impacted.
Context
Step 1 for #928. See CI results at ItsHarper#4.
Description
This PR updates
pnpmto the latest version (10.14.0). That version will automatically be used as long as your globalpnpmis at least version 10, so as this PR documents inCONTRIBUTING.md, make sure to update that.Since you have no particular reason to trust me, I would highly recommend verifying my
pnpm-lock.yamlchanges like so:editso that it pauses after each onegit checkout HEAD~1 -- pnpm-lock.yamlpnpm installgit add pnpm-lock.yamlpnpm, with no hand-editing.git rebase --continue