-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add package analysis with local and tarball dependency reporting to CLI #3
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
Conversation
bluwy
left a comment
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.
LGTM 👍 A couple things I notice but I don't think it's urgent since this project is still in early stages:
analyze-dependencies.tsseems to import node builtin modules, so the library itself can't be used in the browser.- Is the commented out
// this.loglines for debugging only or wip? Maybe good to comment at the top of the file if it's the former. - The dimmed messages under
Tarball Analysisare a bit hard to read on my terminal - The tarball files printed could be a bit long and interfere with the logging. Maybe we could add a hint like
npm pack --dry-runfor users to check themselves.
|
Thank you for the review @bluwy, I will try to address your comments on this PR so they don't get lost in time & space 😁 |
43081j
left a comment
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.
sorry this took so long
looks good to me now. we can iterate further in new branches 👍
This PR adds a dependency analyzer to the CLI, enabling detailed package analysis in two ways:
Local Analysis:
node_modulesTarball Analysis:
I've also added tests for the dependency analysis utilities and improved code documentation.
Related: #2