-
-
Notifications
You must be signed in to change notification settings - Fork 48
Yarn v3 #119
Conversation
39bf28d
to
14d33f3
Compare
f89aa51
to
9b57166
Compare
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 job on this, just had a few suggestions.
Co-authored-by: Elliot Winkler <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
.github/workflows/lint-test.yml
Outdated
@@ -29,8 +29,7 @@ jobs: | |||
with: | |||
path: ${{ steps.yarn-cache-dir.outputs.YARN_CACHE_DIR }} | |||
key: yarn-cache-${{ runner.os }}-${{ steps.yarn-version.outputs.YARN_VERSION }}-${{ hashFiles('yarn.lock') }} | |||
- run: yarn --frozen-lockfile | |||
- run: yarn allow-scripts | |||
- run: yarn --immutable --immutable-cache |
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.
Hmm... maybe we don't want --immutable-cache
after all, since it seems to fail if the cache folder doesn't exist (see CI). @Gudahtt What do you think?
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.
The --immutable-cache
flag was meant to be used in the post-prepare jobs, where it was guaranteed the cache would be populated. Agreed that we shouldn't use it here.
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.
Agh, my bad, that's right. Sorry @adonesky1 we don't want this after all, so this should just be yarn --immutable
😅
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.
done here
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.
LGTM! Couple of minor suggestions but this all looks good
This file has not been used as of the Yarn v3 migration in #119. It was accidentally left behind.
Upgrades package manager from Yarn v1 to Yarn v3.
Followed the migration guide + some of the example set in this migration PR for
@metamask/controller