-
-
Notifications
You must be signed in to change notification settings - Fork 154
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
Switch to pnpm #1063
Switch to pnpm #1063
Conversation
165a65a
to
60a0a28
Compare
@@ -0,0 +1,3 @@ | |||
packages: | |||
- addon | |||
- test-app |
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.
nitpick
At this stage, we don't have the test-app
package yet. I'd have added line 3 in #1062. We can merge this PR as is, since (I think) pnpm
ignores entries corresponding to packages that don't exist yet.
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.
nitpick
In #1060, I noticed you had to update the volta
entry in the root package.json
.
Lines 15 to 18 in 5862d31
"volta": { | |
"node": "16.20.0", | |
"yarn": "1.22.19" | |
} |
I'm guessing that, in this pull request, the entry should have similarly been updated again, by replacing yarn
with pnpm
? (Maybe this happens in a later PR. At any rate, we can merge the PR as is.)
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.
Nice catch! Thanks!
For conflict management, i'd like to do that in the test-app extraction pr.
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.
09e5b5b
to
e6166af
Compare
75d5eae
to
372933e
Compare
Make gitignore work for all directories Add root package.json Update CI Use previously upgraded node
372933e
to
fcc91c8
Compare
Context, Plan, etc: https://gist.github.com/NullVoxPopuli/eafc7dad6547de5e730098498b829e1f
Requires that this be merged first:
Due to errors discovered in the test-app extraction PR, here: #1061
It was determined that yarn is insufficient for ember-qunit.
This PR migrates the repo to pnpm.
Afterwards, the following are unblocked (in-order):