-
Notifications
You must be signed in to change notification settings - Fork 594
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
[package-extractor]Add dependencies configurations field in package extractor #4255
[package-extractor]Add dependencies configurations field in package extractor #4255
Conversation
99947bb
to
0b50293
Compare
Hi @D4N14L, could you please help to review this PR? |
...actor/src/tests/package-extractor-test-repo/packages/foo/node_modules/.pnpm/baz/package.json
Outdated
Show resolved
Hide resolved
I just merged my PR which should address your concerns: #4259 I'm a bit iffy on adding general 3rd party pattern include/exclude support given that it's a bit ambiguous if it applies only to the version of the package consumed directly by a project, or another dependency, or otherwise. The original issue should be addressed by my PR though. |
I will rebase my branch~ But seems this PR can't solve my original issue, the symlink(which linked to global path) inside third party dependencies still cause errors :( |
40ce627
to
51a3006
Compare
Hi @D4N14L , what I need to do if I want to get this PR merged? |
@EscapeB I'm so sorry that I've let this slide. I've been pretty busy of late and just haven't had a chance to take a closer look. Plan on doing so tomorrow when I can find a moment. Again, apologies for holding this up. |
…r files inside node_modules
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
f4340d2
to
7baa41c
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.
Looking good! Just a few more changes then I think it's good to merge.
Co-authored-by: Daniel <3473356+D4N14L@users.noreply.github.com>
Hi @D4N14L, I think all the changes mentioned in the comments have now been made, please help merge this PR when you have time. |
Summary
Add "dependenciesConfigurations" field in options.
Related discussion in #4210
Details
How it was tested
Added some unit test cases to test the new field.