-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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(repository): honour handleTransparentWorkspaces setting in Yarn/Berry #9626
fix(repository): honour handleTransparentWorkspaces setting in Yarn/Berry #9626
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@romanofski is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
Would appreciate some guidance whether an additional integration test would be welcome. |
d183f26
to
7cb26de
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.
Thank you so much for the PR! This looks really good. I made some minor changes/code moving in romanofski#1. Feel free to merge it in or I can do it in a follow up PR.
Only thing blocking right now is fixing the default logic when parsing an yarnrc that doesn't have an enableTransparentWorkspaces
field.
Would appreciate some guidance whether an additional integration test would be welcome.
For this case I think we have enough unit test coverage that adding an integration test isn't necessary.
assert_eq!( | ||
empty, | ||
YarnRc { | ||
enableTransparentWorkspaces: true |
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.
This currently fails as the serde definition requires this field to be present. To make it optional you can either:
- change the type to
Option<bool>
- add
#[serde(default = default_fn)]
to the field: docs
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.
Thanks again @chris-olszewski - learned something. Also for the improvements. Was able to learn from them too :)
979fbe1
to
7cb26de
Compare
7dbad5d
to
00f40b6
Compare
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
1 similar comment
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
Description
This implements a fix for #8989 to honour
enableTransparentWorkspaces
in a yarn config.The patch folds the config setting into the existing
link_workspace_packages
npm settings as they're equivalent.Massive Kudos to @me4502 as she helped throughout the implementation process with testing and figuring things out.
Testing Instructions
a
:turborepo/target/debug/turbo build --force --skip-infer --no-daemon --filter a