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

deps: increase "engines" to "node" : "^12.22 || ^14.13 || >=16" #2601

Merged
merged 1 commit into from Jan 31, 2022

Conversation

wraithgar
Copy link
Contributor

@wraithgar wraithgar commented Jan 27, 2022

Makes npm warn users if they are using an unsupported Node version.

Checklist
Description of change

Commit message follows the pattern from the last time this field was updated: #2153

Makes npm warn users if they are using an unsupported Node version.
@richardlau
Copy link
Member

@richardlau richardlau commented Jan 27, 2022

I think we usually treat dropping support for a version (in this case Node.js 10) as semver-major.

@wraithgar
Copy link
Contributor Author

@wraithgar wraithgar commented Jan 27, 2022

That was my first instinct but then I checked the git history on that line and that was not done last time. In the cli we are treating this kind of change as a semver major, and would happily amend my commit.

The current testing matrix only includes 12.x, 14.x and 16.x which I believe only uses the latest versions in those lines.

@richardlau
Copy link
Member

@richardlau richardlau commented Jan 27, 2022

I don't feel strongly enough about this to argue either way so I'll remove the label -- this specific change is is advisory (at least for npm, I think some (all?) versions of yarn enforced engines by default).

Given that we don't actively test against Node.js 10 we can't even say that we aren't already broken there (albeit I'm sure we'd have received plenty of reports if that been the case).

If other node-gyp maintainers disagree, feel free to put back the semver-major label.

@rvagg
Copy link
Member

@rvagg rvagg commented Jan 27, 2022

let's do semver-major !, it's not a big deal to bump that, releases are relatively cheap and signalling is important

@wraithgar do you have any urgency about getting a release for this out for consumption upstream?

@wraithgar
Copy link
Contributor Author

@wraithgar wraithgar commented Jan 29, 2022

@rvagg I'd say medium low urgency? It's mostly about allowing node-gyp to stay current w/ the latest make-fetch-happen. v10 has a couple of bug fixes in it, nothing very big (yet). It would also shrink the npm cli bundle size a bit by allowing us to dedupe make-fetch-happen up out of node-gyp.

@rvagg rvagg merged commit 6562f92 into nodejs:master Jan 31, 2022
28 checks passed
@github-actions github-actions bot mentioned this pull request Jan 31, 2022
@wraithgar wraithgar deleted the gar/drop-node-10 branch Jan 31, 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