Skip to content
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

fix(core): Correct yarn version detection #9005

Closed
wants to merge 2 commits into from
Closed

fix(core): Correct yarn version detection #9005

wants to merge 2 commits into from

Conversation

0420syj
Copy link
Contributor

@0420syj 0420syj commented May 24, 2023

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

I checked PR #8908.
However, the presence of a .yarnrc.yml file alone is not a definitive indicator of Yarn 2+ being used, as this file can be used by Yarn 1.x as well.

Check out the capture image below

screenshot1

Test Plan

  • Check out an older docusaurus project and copy the file in place.
  • Install the project with npm
  • Run a npm run start
  • Close and run it again
    • Confirm the notice had an appropriate npm i command
  • rm -rf node_modules
  • Install with yarn classic
  • Run yarn start
  • Close and run it again
    • Confirm the notice had an appropriate npm i command
  • rm -rf node_modules
  • Repeat with yarn 2

Test links

Deploy preview: https://deploy-preview-9005--docusaurus-2.netlify.app/

Related issues/PRs

#8908

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 24, 2023
@0420syj 0420syj closed this May 24, 2023
@0420syj 0420syj deleted the correct-yarn-version-detection branch May 24, 2023 14:29
@slorber
Copy link
Collaborator

slorber commented May 30, 2023

Hi

Wonder what made you close this PR. Is the problem still relevant? Wasn't it a good solution?

cc @andrewnicols

@0420syj
Copy link
Contributor Author

0420syj commented May 30, 2023

@slorber
Hi
I closed this PR for my simple mistake. I made little improvement compare to this one in #9006.

I thought I should make only 1 commit in 1 PR, so I opened new PR. The moment I noticed that I could have just squash and force push, I have already created new PR. Sorry for that happended 😅

@slorber
Copy link
Collaborator

slorber commented May 30, 2023

I see thanks didn't notice the other PR yet 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants