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

Set min supported node at 16 #56

Merged
merged 1 commit into from
Jul 1, 2022
Merged

Set min supported node at 16 #56

merged 1 commit into from
Jul 1, 2022

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jun 30, 2022

nextcloud/standards#1

Node 16 ships npm 8 since 16.11.0.
I suggest we just leave npm to 7.0.0 as this is the minimum supported. Devs can still use npm 8 if they want :)

Everyone on board?

@max-nextcloud @vinicius73 @artonge @CarlSchwan @JuliaKirschenheuter @GretaD @jotoeri @raimund-schluessler @korelstar @marcoambrosini @eneiluj

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added the dependencies Pull requests that update a dependency file label Jun 30, 2022
@skjnldsv skjnldsv self-assigned this Jun 30, 2022
@skjnldsv skjnldsv changed the title Update dispatch-npm-engines.yml Set min supported node at 16 Jun 30, 2022
@korelstar

This comment was marked as off-topic.

@skjnldsv

This comment was marked as off-topic.

@jotoeri

This comment was marked as off-topic.

@skjnldsv
Copy link
Member Author

skjnldsv commented Jun 30, 2022

Default is the wrong word. Min-supported is more fitting.

Sorry, this is not up to debate here ^^"
This issue is about raising from 14 to 16 prematurely

Nonetheless we can also discuss about raising min-NPM to 8 as well, removing support for 7. But I don't see the reasoning behind this as node16 ships both npm 7 and 8

Please read nextcloud/standards#5

Copy link
Member

@jotoeri jotoeri left a comment

Choose a reason for hiding this comment

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

Ok, so talking only about the node version, i'm fine with this. 😉

@jotoeri

This comment was marked as off-topic.

@raimund-schluessler
Copy link
Member

I am fine with raising the node version to 16.

On a side note, i would also prefer no warnings when using a supported npm version 😉

@skjnldsv skjnldsv requested a review from vinicius73 June 30, 2022 20:02
@skjnldsv skjnldsv merged commit 6a8fefb into master Jul 1, 2022
@skjnldsv skjnldsv deleted the feat/npm7-node16 branch July 1, 2022 08:47
@skjnldsv
Copy link
Member Author

skjnldsv commented Jul 1, 2022

Devs can still use npm 8 if they want :)

But this will give warnings, since npm 8 is not compatible to ^7.0.0. What about setting NPM_VERSION: "^7.0.0 || ^8.0.0", then all node@16 installations (e.g. official docker container) will work just out of the box?

Discussion in nextcloud/standards#7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants