-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: [OSM-1024] pnpm dep graph builder #218
Conversation
317b87f
to
45b9480
Compare
cd7d94f
to
d4f3f4a
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.
Very well done!!!
Left some minor comments, great job!
1d9aecf
to
1605d2d
Compare
Great job, it's looking good 👍 I hope my draft PR was a bit helpful :) You might want to consider to also support lock file v7 is will be the basis for the upcoming pnpm v9. Currently in beta. You could convert the v7 back into v6 format and use that for the rest of the code.
Yups, that's correct. I added the pnpm support for Gitlab. |
1605d2d
to
aa18731
Compare
b70eaeb
to
05f7565
Compare
Thanks @weyert ! 😄 I saw your PR and wanted to link my changes here. Really appreciate your work and taking the time to look over this. As for pnpm v9, I think we'll take it into account once it's not beta, should the new lockfile version not be backwards compatible. |
@gemaxim It's definitely not backward compatible, main reason for the major version bump actually, some of the changes:
|
259c460
to
bc70348
Compare
b881ea2
to
01baca2
Compare
01baca2
to
384a1f7
Compare
🎉 This PR is included in version 1.53.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Great work 👍 |
What this does
Extends functionality with dependency graph builder for pnpm lockfiles with versions 5.x and 6.x (corresponding to pnpm@7 and pnpm@8).
Notes for the reviewer
Check out the added fixtures for pnpm lockfile v5 and pnpm lockfile v6. 'package.json' inputs taken from npm and yarn fixtures and checked for the same expected dependency graph.
pnpm lockfiles do not store bundled dependencies. (related to 2 test cases). Links about this: https://docs.gitlab.com/ee/user/application_security/dependency_scanning/ ,
bundledDependencies: false
inconsistently added to pnpm-lock.yaml pnpm/pnpm#7576.Ported a few related errors to error catalog (only related to new functionality, as we still need to see if cli correctly handles them), need to update @snyk/error-catalog-nodejs-public for tests to pass.
More information