Skip to content

use node version manager - #188

Merged
gengjiawen merged 3 commits into
react-native-community:masterfrom
krudos:use-nvm
Nov 25, 2022
Merged

use node version manager#188
gengjiawen merged 3 commits into
react-native-community:masterfrom
krudos:use-nvm

Conversation

@krudos

Copy link
Copy Markdown
Contributor

No description provided.

@gengjiawengengjiawen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you want to add a node version manager, I would prefer n. This would be more less intrusive and easy (use npm to install n)

Comment threadDockerfile Outdated
@krudos
krudos marked this pull request as draft November 22, 2022 09:33
@krudoskrudos changed the title use nvmuse node version managerNov 22, 2022
@krudos
krudos marked this pull request as ready for review November 22, 2022 09:34
Comment threadDockerfile

@gengjiawengengjiawen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@cortinico

Copy link
Copy Markdown
Member

I would refrain from adding a node version manager in the container frankly. Version managers are user tools. The idea behind a container is to have a fixed set of dependencies that are used to build a given tool.

@gengjiawengengjiawen mentioned this pull request Nov 23, 2022
@gengjiawen

Copy link
Copy Markdown
Member

I would refrain from adding a node version manager in the container frankly. Version managers are user tools. The idea behind a container is to have a fixed set of dependencies that are used to build a given tool.

It only added one npm package in the end, the cost is very small. In the long run, I think this is very useful for user-land to do matrix testing(Node.js latest, lts, minimum)

@leotm

Copy link
Copy Markdown
Contributor

agree w a minimal container but also the small cost of n (or some other hip manager, not talking big o notation)

node 19's out (working for me) and others testing matrices ahead's good for long-term contribution (node 20 next easter)

noticed ubuntu-22.04 comes ootb w: node 18, n and nvm (node 12 removed from all images last wk)

@gengjiawen
gengjiawen merged commit 109dfce into react-native-community:masterNov 25, 2022
@gengjiawen

Copy link
Copy Markdown
Member

Thx for contribution

Sign up for freeto 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.

4 participants

@krudos@cortinico@gengjiawen@leotm