Conversation
styfle
left a comment
styfle
left a comment
There was a problem hiding this comment.
Looks good to me, just needs tests 👍
Added! Let me know if you think there's more I should check for - there's not much actual new-new code here so hopefully we're good. Also LMK if you have thoughts on the follow-ups I listed or if you think there should be others. I could get started on some of them. |
Co-authored-by: Steven <steven@ceriously.com>
|
Can you add some documentation please? |
There was a problem hiding this comment.
npm is always lowercase https://npmjs.com/package/npm#faq-on-branding
specifically: - assert that `stdout` doesn't contain the word "Error" - use the same npm/pnpm/yarn versions that are already used elsewhere, maybe some mocking is happening - add a license field - check for existence of node_modules/ms/package.json
|
Fixed the tests - they had been working at some point but seemed to have started failing downloading the versions of npm/pnpm/yarn I'd specified, so I just changed them to match versions that are used elsewhere in tests. Also fixed the NPM->npm casing issue. @anonrig if you could give another CI approval (I think) this is good to go! |
|
It looks like there are some linting errors to address. |
|
After reading the updated document in this PR, I wonder if |
10 participants
Closes #505
This adds a
corepack project installcommand, which is roughly equivalent tocorepack enable && npm installfor npm projectscorepack enable && pnpm installfor pnpm projectscorepack enable && yarn installfor yarn projectsI was able to use it successfully by just running
yarn buildlocally, then I could donode dist/corepack.js project installto verify it works on the corepack repo itself, andnode ../corepack/dist/corepack.js project installfrom various git repos next door.I wanted to open this up early to get feedback in case the direction isn't considered correct. If it's looking good, I will add tests along similar lines to the existing ones.
Some follow-ons (which I think could be implemented as separate PRs, but could be persuaded to roll them into this one)
--frozen-lockfileand--no-frozen-lockfile--lockfile-only--no-lockfile--production--dev--offline--forcecorepack project add left-padcorepack project remove left-padCC @styfle and @aduh95 since you commented on the issue.