Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: upgrade to yarn 3#6162

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
JoshuaKGoldberg merged 3 commits intomainfromyarn-3
Aug 20, 2023
Merged

chore: upgrade to yarn 3#6162

JoshuaKGoldberg merged 3 commits intomainfromyarn-3
Aug 20, 2023

Conversation

JamesHenry
Copy link
Member

@JamesHenryJamesHenry commentedDec 3, 2022•
edited
Loading

PR Checklist

Overview

Migrates the codebase from yarn v1 to yarn v3, using thenode_modules linker -not PnP - to ensure maximum consistency before and after the migration.

I confirmed that there were no issues with lerna publishing by publishing a canary version to verdaccio locally (this will not negatively impact the canary release post-merge):

image

omril1, vuki656, and armano2 reacted with rocket emoji
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@JamesHenry!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint.

@netlify
Copy link

netlifybot commentedDec 3, 2022•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit7d2396e
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/64e11b877623d60008e113be
😎 Deploy Previewhttps://deploy-preview-6162--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site configuration.

@JamesHenryJamesHenryforce-pushed theyarn-3 branch 2 times, most recently from6bed2f3 tod8a5d74CompareDecember 3, 2022 14:05
@JamesHenry
Copy link
MemberAuthor

@bradzacher@JoshuaKGoldberg please have a play with this, it all looks good to me locally and CI is passing.

Once you are happy and approve the PR pleasedo not merge, so that I can coordinate the update of the automation jobs

JoshuaKGoldberg reacted with thumbs up emoji

JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesDec 3, 2022
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks great, thanks! 🔥

bradzacher
bradzacher previously approved these changesDec 7, 2022
Copy link
Member

@bradzacherbradzacher left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

25a

once you fix the CI I'm happy with this.
now i'll need to get used to the new console output from yarn sadge

JamesHenry and JoshuaKGoldberg reacted with laugh emoji
bradzacher
bradzacher previously approved these changesDec 12, 2022
bradzacher
bradzacher previously approved these changesDec 14, 2022
@liuxingbaoyu
Copy link
Contributor

Thanks for all this!

JoshuaKGoldberg reacted with heart emoji

@bradzacherbradzacher added repo maintenancethings to do with maintenance of the repo, and not with code/docs 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge labelsJan 30, 2023
@nx-cloud
Copy link

nx-cloudbot commentedJul 30, 2023•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit7d2396e. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 40 targets

Sent with 💌 fromNxCloud.

@@ -52,7 +52,11 @@
},
"devDependencies": {
"@typescript-eslint/parser": "6.2.0",
"ajv": "^8.12.0",
Copy link
MemberAuthor

@JamesHenryJamesHenryJul 30, 2023•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I had to downgrade this to match the rest of the workspace, otherwisepatch-package hard errors on it

@@ -78,6 +78,10 @@
},
"devDependencies": {
"@typescript-eslint/parser": "6.2.0",
"downlevel-dts": "*",
"jest": "29.6.1",
Copy link
MemberAuthor

@JamesHenryJamesHenryJul 30, 2023•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Having to match all thesejest versions is pretty cumbersome, but required to make the nakedjest calls in npm scripts work with modern yarn.

The way they recommend handling this is different and involves sharing scripts between workspaces, or usingrun -T in front of the script contents, see here:

https://yarnpkg.com/getting-started/qa#how-to-share-scripts-between-workspaces

Somewhat related - we could avoid the need for this entirely if we always let Nx run the tasks through its own executors and then just switch the script contents to benx test like we did here:

"test":"nx test --code-coverage",
- it would resolvejest for us, and we wouldn't need to bend over backwards to make yarn happy

JoshuaKGoldberg reacted with thumbs up emoji

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

That sounds good to me! IIRC you'd mentioned this as a potential followup a while back. I'm definitely in favor.

@@ -76,15 +76,19 @@
"@types/prettier": "*",
"@typescript-eslint/rule-schema-to-typescript-types": "6.2.0",
"@typescript-eslint/rule-tester": "6.2.0",
"ajv": "^6.12.6",
"ajv": "^6.10.0",
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Changed to match other references

JoshuaKGoldberg reacted with thumbs up emoji
@JamesHenryJamesHenry removed the awaiting responseIssues waiting for a reply from the OP or another party labelJul 30, 2023
@JamesHenryJamesHenry mentioned this pull requestJul 30, 2023
1 task
@@ -72,6 +72,9 @@ jobs:
rm migrations.json
fi

# Run the special nx repair command to ensure config matches latest and greatest
npx nx repair

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

(not a blocker, I just am curious what this actually does)

@@ -5,7 +5,9 @@
command = "NX_VERBOSE_LOGGING=true yarn patch-package && yarn nx build website"
[build.environment]
NETLIFY_USE_YARN = "true"
YARN_FLAGS = "--ignore-scripts"
# TODO: adjust these once https://github.com/netlify/build-image/issues/612 is resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

That awkward moment when the upstream repo is archived without resolving the issue... 🤷

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This looks great! I read through and think I understand the strategies and upstream-blocked-todo-workarounds. Also I tried it out locally and -with the caveat I commented inpackage.json- it seems to work great on my end. Nice! 👏

Only requesting changes because I think we'll need to update the local development docs.

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelJul 31, 2023
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelAug 19, 2023
@JamesHenry
Copy link
MemberAuthor

@JoshuaKGoldberg Brought it back up to date and updated the local development guide. Hopefully we can merge now!

JoshuaKGoldberg reacted with rocket emoji

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Tried it out locally again and it all works great! No need for me to manually run.cjs files from the repo. Thanks! Awesome stuff@JamesHenry 🚀

omril1 and armano2 reacted with rocket emoji
@JoshuaKGoldbergJoshuaKGoldberg merged commit2e1cfd5 intomainAug 20, 2023
@JoshuaKGoldbergJoshuaKGoldberg deleted the yarn-3 branchAugust 20, 2023 22:10
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsAug 28, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@fiskerfiskerfisker left review comments

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

@bradzacherbradzacherAwaiting requested review from bradzacher

Assignees
No one assigned
Labels
1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we mergerepo maintenancethings to do with maintenance of the repo, and not with code/docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Repo: Can't develop because of yarn1
5 participants
@JamesHenry@liuxingbaoyu@fisker@JoshuaKGoldberg@bradzacher
[8]ページ先頭
©2009-2025 Movatter.jp