-
-
Notifications
You must be signed in to change notification settings - Fork 298
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
Ensure that Node.js version bumps are done in major releases #686
base: main
Are you sure you want to change the base?
Ensure that Node.js version bumps are done in major releases #686
Conversation
We just ignore it. That's an extreme edge-case anyway.
"Targeting a major Node.js version is a breaking change and requires a major version bump"?
It shouldn't do anything if there are no previous version to compare to. |
@@ -62,6 +63,18 @@ const prerequisiteTasks = (input, pkg, options) => { | |||
newVersion = Version.getAndValidateNewVersionFrom(input, pkg.version); | |||
}, | |||
}, | |||
{ | |||
title: 'Check for Node version bump', |
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.
title: 'Check for Node version bump', | |
title: 'Check for Node.js version bump', |
I agree, I meant more “what does the code do since it doesn’t check for that case”.
Are we only erroring on major Node changes, or any change? And for the target field in |
Any change. Any change to the target Node.js version is a potentially breaking change. |
Bump :) |
I’m waiting until #689 is finished before completing this, as it has a better test suite. I’m close to being done with it. |
Bump :) |
Closes #88. Currently just a scaffold for a new prerequisite task. Related: sindresorhus/read-pkg#28
Some questions:
The Node version has increased, but this release is not a major version.
, but that's not the best. Any ideas for improvements?Related to the first question, it just struck me - what happens with #681 on the first release? I don't think I took that case into account.