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(cli): don't include workspaces.nohoist in public NPM package #22365
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ FlakinessThis comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
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.
Looks good. Is there a way to write a test for this change?
@ryanthemanuel I don't see an obvious way to test it since it's in the build step. I could add an assertion in buiild.js or post-build.js, but it would be overly unit testy imo. I think this script should be updated in general, we shouldn't need to do all this wacky merging of 2 package.jsons, I think all of the should be all in one place as much as possible. So no |
I figured as such. Approved! |
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.
Looks good!
User facing changelog
Fixed an issue causing a "nohoist config is ignored" warning when installing
cypress
with yarn.Additional details
Steps to test
Install latest prerelease comment on this branch on Linux, note there is no warning.
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?