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

Implement global package upgrade #70

Merged
merged 16 commits into from
Jan 20, 2022
Merged

Implement global package upgrade #70

merged 16 commits into from
Jan 20, 2022

Conversation

medallyon
Copy link
Contributor

This PR addresses #8 and implements the ability to supply -g or --global to npm-upgrade.

  • Supplying --global overrides any other supplied flags
  • There is no "global" package.json, so upgrades are applied immediately by calling npm install upon finish
  • The global path to node_modules is determined by running npm root -g. This has been tested on Windows 10 and Ubuntu 20.04.

README.md Outdated Show resolved Hide resolved
@@ -68,7 +78,7 @@ export const handler = catchAsyncError(async opts => {
const filteredWith = filter ? `filtered with ${strong(filter)} ` : '';

console.log(
`Checking for outdated ${depsGroupsToCheckStr}dependencies ${filteredWith}for "${strong(packageFile)}"...`
`Checking for outdated ${depsGroupsToCheckStr}dependencies ${filteredWith}${opts.global ? '' : (`for "${strong(packageFile)}"`)}...`
Copy link
Owner

Choose a reason for hiding this comment

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

Can we show "Checking for outdated global dependencies..." when --global flag is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me it does say this already:
image

src/commands/check.js Outdated Show resolved Hide resolved
src/commands/check.js Outdated Show resolved Hide resolved
src/packageUtils.js Outdated Show resolved Hide resolved
@medallyon medallyon requested a review from th0r November 26, 2021 18:05
This includes fixes to line-endings, brace-style, and max-len.
src/commands/check.js Outdated Show resolved Hide resolved
} catch (err) {
console.error(err);
console.error(`Error parsing global packages: ${err.message}`);
process.exit(1);
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you decide to use process.exit here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to include it to mimic the loadPackageJson method as closely as possible, as it also calls process.exit on attempting to parse the packageJson.

Copy link
Owner

Choose a reason for hiding this comment

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

Any comment?

Copy link
Contributor Author

@medallyon medallyon Dec 15, 2021

Choose a reason for hiding this comment

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

Sorry? I commented on Nov 30. Is it not visible to you?

I decided to include it to mimic the loadPackageJson method as closely as possible, as it also calls process.exit on attempting to parse the packageJson.

Copy link
Owner

Choose a reason for hiding this comment

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

@medallyon any comment about this change?

@medallyon medallyon requested a review from th0r December 7, 2021 04:53
@medallyon
Copy link
Contributor Author

Hi @th0r, could you look at this again sometime? I commented on all of the stuff you wanted clarification for, I don't know if you can see them or not.

@medallyon
Copy link
Contributor Author

@th0r You keep commenting "Any comment on this change?" in the same conversation, to which I've replied twice. I'm assuming you can't see my replies, so here's a screenshot.

image

@th0r
Copy link
Owner

th0r commented Jan 17, 2022

I'm assuming you can't see my replies, so here's a screenshot.

Your replies have a Pending tag - you haven't submitted them. So, yes, I can't see them.

@medallyon
Copy link
Contributor Author

medallyon commented Jan 17, 2022

How do I submit them?? I've replied to all of your reviews as soon as they came in!

@th0r
Copy link
Owner

th0r commented Jan 20, 2022

When I do npm install package-lock.json changes. Could you commit this change please?

@medallyon
Copy link
Contributor Author

Your replies have a Pending tag - you haven't submitted them. So, yes, I can't see them.

I found the "Submit Review" button 👺

@th0r th0r merged commit 9d107bc into th0r:master Jan 20, 2022
@th0r
Copy link
Owner

th0r commented Jan 20, 2022

Merged, thanks a lot! I'll publish a new version in a few hours.

@th0r
Copy link
Owner

th0r commented Jan 21, 2022

Published in v3.1.0

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

2 participants