-
Notifications
You must be signed in to change notification settings - Fork 58
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: handle cases where no package.json and global deps #471
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
🦋 Changeset detectedLatest commit: 40746f9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #471 +/- ##
==========================================
+ Coverage 79.74% 79.85% +0.10%
==========================================
Files 75 75
Lines 6029 6060 +31
Branches 603 606 +3
==========================================
+ Hits 4808 4839 +31
Misses 1220 1220
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
8edbb6b
to
40746f9
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.
Will this work with Yarn?
Not sure, Would likely need to add another case. I dont have time to work on this more today, so you can merge or update if you need |
let dependencies: Dependencies = {}; | ||
|
||
// Attempt to get all globally installed pacakges. | ||
const result = sync('npm', ['list', '-g', '--json', '--depth=0']); |
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.
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.
Thanks @SimenB, I think I'll just remove dependency checks, there are too many variants to consider given the value of this feature
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.
@SimenB this will be removed in v0.45.0, please let me know if that works for you once you try it out
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.
Yeah, 0.45 works. Thanks!
closes: #469