这是indexloc提供的服务,不要输入任何密码
Skip to content

Add yarn berry node_modules linker, CRLF/LF, and update prune #544

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

Merged
merged 29 commits into from
Jan 14, 2022
Merged

Add yarn berry node_modules linker, CRLF/LF, and update prune #544

merged 29 commits into from
Jan 14, 2022

Conversation

samchouse
Copy link
Contributor

@samchouse samchouse commented Jan 11, 2022

closes #471
closes #536

partially fixes #237 (only node_modules linker)

Todo:

  • Update docs
  • Finish reviewing source code
  • Finish Yarn Berry implementation
  • Support CRLF on Unix and LF on Windows

What this PR brings:

  • Fix some typos
  • Add Yarn Berry support
  • Cleanup dead & duplicate code

Cleaning up the dead code was done by StaticCheck so if any parts of the code should be held onto for future use, I will revert that change.

@vercel
Copy link

vercel bot commented Jan 11, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/5bQhp3DQBLvEcPpS3JQH8sCiS9ST
✅ Preview: https://turbo-site-git-fork-xenfo-main.vercel.sh

@jaredpalmer
Copy link
Contributor

jaredpalmer commented Jan 11, 2022

Looking good! Keep it going!

@samchouse
Copy link
Contributor Author

samchouse commented Jan 11, 2022

@jaredpalmer I wrote an E2E test for Berry and I noticed that each time I run it, the results vary without changing files. A is impacted randomly, B is impacted randomly, sometimes not at all always with a random package manager. Is this normal?

Edit: This actually applies to all tests, running make e2e many times while changing nothing produces different results.

@jaredpalmer
Copy link
Contributor

I noticed test flakiness over the past week or so. Haven't had a chance to look deeper

@samchouse
Copy link
Contributor Author

What's the point of iswin_windows.go and iswin_other.go?

@samchouse samchouse changed the title fix: cleanup + yarn berry fix: qol + cleanup + yarn berry Jan 12, 2022
@jaredpalmer
Copy link
Contributor

monorepo.linkPackages is failing in e2e for berry https://github.com/vercel/turborepo/runs/4790317239?check_suite_focus=true#step:7:73

@samchouse
Copy link
Contributor Author

monorepo.linkPackages is failing in e2e for berry https://github.com/vercel/turborepo/runs/4790317239?check_suite_focus=true#step:7:73

Not sure why e2e is fine on my machine.

@samchouse
Copy link
Contributor Author

I don't know how to support PNP so this PR will only fix NM linker. I saw some stuff in the install script but it doesn't seem to be working.

turbo@npm:1.0.26 couldn't be built successfully (exit code 1, logs can be found here: /tmp/xfs-d56745cc/build.log)

@jaredpalmer
Copy link
Contributor

jaredpalmer commented Jan 12, 2022

What's the point of iswin_windows.go and iswin_other.go?

Legacy. Can be removed

Co-authored-by: Jared Palmer <jared@jaredpalmer.com>
@jaredpalmer jaredpalmer changed the title fix: yarn berry node_modules linker, CRLF/LF, and prune Add yarn berry node_modules linker, CRLF/LF, and prune Jan 13, 2022
@jaredpalmer jaredpalmer changed the title Add yarn berry node_modules linker, CRLF/LF, and prune Add yarn berry node_modules linker, CRLF/LF, and update prune Jan 13, 2022
@jaredpalmer jaredpalmer mentioned this pull request Jan 13, 2022
2 tasks
@jaredpalmer jaredpalmer merged commit 6a5d70e into vercel:main Jan 14, 2022
@jaredpalmer
Copy link
Contributor

jaredpalmer commented Jan 14, 2022

I lied. I think we should not throw if packageManager is not defined (yet). Let's add old detection code as fallback for now and add a warning to with instructions to run "npx @turbo/migrate add-package-manager"

@samchouse
Copy link
Contributor Author

How do I issue a warning?

@jaredpalmer
Copy link
Contributor

logWarning() or log.Println("[WARNING] ...")

@samchouse
Copy link
Contributor Author

samchouse commented Jan 14, 2022

Where do I get those functions? Do I warn in nodejs.go or run.go, etc.?

@samchouse
Copy link
Contributor Author

@jaredpalmer Still not sure how to warn from the backend file.

@jaredpalmer
Copy link
Contributor

log is a standard Go package https://pkg.go.dev/log

@samchouse
Copy link
Contributor Author

Sorry, it wasn't printing anything before 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants