Conversation
e82fe06 to
7934e31
Compare
jakecoffman
approved these changes
Aug 26, 2025
7934e31 to
bf3544b
Compare
bf3544b to
af98120
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
This branch follows up on our work to date on the
enable_dependency_submission_pocexperiment to incorporate the changes we require to lockfile parsing to npm'spackage-lock.jsonso that we can accurately graph a project using this file.This incorporates the things that we are using to interpret a Dependabot parse for snapshots which are:
For npm, this has a slight wrinkle which is that we need to make a call out to
npm lsto reliably determine the top-level dependencies for a project since only v3 lockfiles provide this information.We could do this in a different way by comparing information across dependency files but that cuts against our idiom so far so I've avoid breaking/changing the pattern for now. I've limited this new native command to only runs with our experiment enabled to avoid side-effects.
Anything you want to highlight for special attention from reviewers?
Our approach is still on a 'make it work' basis to avoid making any assumptions about how we abstract a 'normal' parse from a 'verbose' parse for graphs.
Once we have a critical few package managers working, we will take a long look at our strategy and determine how to course correct before we fan out across all package managers.
Other changes
The first commit on this branch backfills a
FileParser-level test for Bundler. We've been using Bundler to drive the updater/integration tests but it was useful to backfill a Bundler test at this level as a reference for the new work required for npm.How will you know you've accomplished your goal?
I can generate a snapshot for a typical npm project that has all the same information a source-based parse provides.
Checklist