-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/vercel/turbo-site/5bQhp3DQBLvEcPpS3JQH8sCiS9ST |
Looking good! Keep it going! |
@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 |
I noticed test flakiness over the past week or so. Haven't had a chance to look deeper |
What's the point of |
|
Not sure why e2e is fine on my machine. |
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.
|
Legacy. Can be removed |
Co-authored-by: Jared Palmer <jared@jaredpalmer.com>
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" |
How do I issue a warning? |
|
Where do I get those functions? Do I warn in |
@jaredpalmer Still not sure how to warn from the backend file. |
|
Sorry, it wasn't printing anything before 😅 |
closes #471
closes #536
partially fixes #237 (only node_modules linker)
Todo:
What this PR brings:
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.