Add a .nvmrc file and update the README with info about NVM #3012
Conversation
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @stefanalfbo)
a discussion (no related file):
Not blocking but not accepting, but after many experiences throughout different project my current stance is to avoid committing versioning files to repositories. Reasons are multiple, for few examples: what should ASDF users do? What if somebody wants to try to build with latest/custom Node version?
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @stefanalfbo)
a discussion (no related file):
Previously, mkaput (Marek Kaput) wrote…
Not blocking but not accepting, but after many experiences throughout different project my current stance is to avoid committing versioning files to repositories. Reasons are multiple, for few examples: what should ASDF users do? What if somebody wants to try to build with latest/custom Node version?
My personal suggestion is to gitignore this file on your machine :-)
|
Interesting, I see the version of Node as a dependency as any other dependency that a project has, and NVM makes it convenient to switch to the correct version. It's not forcing anyone to use it, like the README-file. But I totally see your point though and the cons with adding one more configuration file. However my subjective experience with NVM has only been nice so far 😄 Thanks for fast feedback! |
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @stefanalfbo)
a discussion (no related file):
Interesting, I see the version of Node as a dependency as any other dependency that a project has, and NVM makes it convenient to switch to the correct version. It's not forcing anyone to use it, like the README-file.
Sound like what you want to achieve here is to the engine field on package.json?
https://docs.npmjs.com/cli/v9/configuring-npm/package-json#engines
As I understand it the engines-section in the package.json is not intended for this and it is enough to set the version for the vscode-property in this project. The .nvmrc-file would only define which node version to use during development of the extension and not the version of the node that will consume the vscode extension package (which the node-property is intended for in other types of projects). Feel free to close this PR, and if the need of NVM would emerge in the future then I'm happy to help! |
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @stefanalfbo)
a discussion (no related file):
Previously, mkaput (Marek Kaput) wrote…
Interesting, I see the version of Node as a dependency as any other dependency that a project has, and NVM makes it convenient to switch to the correct version. It's not forcing anyone to use it, like the README-file.
Sound like what you want to achieve here is to the
enginefield onpackage.json?https://docs.npmjs.com/cli/v9/configuring-npm/package-json#engines
@orizi @spapinistarkware I'm leaving the decision up to you :)
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @orizi and @stefanalfbo)
a discussion (no related file):
Previously, mkaput (Marek Kaput) wrote…
@orizi @spapinistarkware I'm leaving the decision up to you :)
If this make sit more approachable to newcomers, I'm all for it.
vscode-cairo/README.md line 3 at r1 (raw file):
# Installation Use [NVM](https://github.com/nvm-sh/nvm#installing-and-updating) to get the correct version of Node for this project. From the directory of this file, run:
Make the line at most 100 chars.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @orizi)
This PR will solve the issue #3011.
This change is