Skip to content
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: add global getter to npm class #4935

Merged
merged 1 commit into from May 25, 2022
Merged

Conversation

nlf
Copy link
Contributor

@nlf nlf commented May 24, 2022

currently everywhere we were doing this.npm.config.get('global') was not accounting for the location flag, so keeping in tradition with our other "derived" configs i
attached it as a getter to the npm class and changed all the references to use the getter

@nlf nlf requested a review from a team as a code owner May 24, 2022 17:52
@@ -343,7 +343,9 @@ t.test('should return if no outdated deps', async t => {
})

await outdated(testDir, {
global: false,
config: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tests were passing entirely by coincidence. if they had been setting global to true they would not have done the expected thing

Copy link
Collaborator

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@lukekarrys lukekarrys merged commit 0f89e07 into latest May 25, 2022
@lukekarrys lukekarrys deleted the nlf/add-global-getter branch May 25, 2022 16:56
@lukekarrys lukekarrys mentioned this pull request May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants