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: make "resolve" external, fixes pnp #165

Merged
merged 4 commits into from
Jul 13, 2022
Merged

Conversation

goloveychuk
Copy link
Contributor

fixes #163

@goloveychuk goloveychuk requested review from styfle and Timer as code owners July 8, 2022 14:19
@goloveychuk
Copy link
Contributor Author

@guybedford could you please check?

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could you add a test to make sure this works as you expect?

@goloveychuk
Copy link
Contributor Author

I would need to add new github check, create a dir with yarn3 project, ran yarn install. Is it ok?

@goloveychuk
Copy link
Contributor Author

Also would need to pack plugin to tar file. And reference it from yarn3 package. Not straightforward.

@styfle
Copy link
Member

styfle commented Jul 11, 2022

I think it’s best to add a test. Otherwise how do we know it works? And how do we prevent regressions in the future? Imagine bumping the dependency version.

@goloveychuk
Copy link
Contributor Author

@styfle added, please approve a run

@styfle
Copy link
Member

styfle commented Jul 13, 2022

Thanks! Looks like windows is failing CI

@goloveychuk
Copy link
Contributor Author

goloveychuk commented Jul 13, 2022

I've made it run only on linux, please reapprove. Failing probably because of bash scripts

@styfle
Copy link
Member

styfle commented Jul 13, 2022

Probably because env vars work differently on Windows. Running on linux only is a fine workaround, thanks!

@styfle styfle enabled auto-merge (squash) July 13, 2022 15:44
@styfle styfle merged commit d22269d into vercel:main Jul 13, 2022
@github-actions
Copy link

🎉 This PR is included in version 1.7.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@styfle
Copy link
Member

styfle commented May 16, 2024

@goloveychuk I had to disable this test when upgrading dependencies here:

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

Successfully merging this pull request may close these issues.

Yarn berry pnp support
2 participants