-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Change .nvmrc and documentation for Node.js version (LTS to 14.18.1) #36744
Conversation
There is another PR opened nearly 2 weeks ago which pins node to 14.x line rather than to the specific version which is going to change quickly. See #36384 from @walbo. I think the only blocker for using LTS is the fact that Node 16.x that was promoted to LTS a few weeks ago is using npm v8 rather than npm v6 that we still have to use. I recently noticed that many projects use |
@gziolo If I change to v14, will it allow us to get this in sooner? Every time I face issues with this and I have to imagine others do as well. This also updates the all important docs to better reflect the versions. |
Yes, it's also what @youknowriad did for https://github.com/WordPress/gutenberg/blob/wp/5.8/.nvmrc and what we did in the past (version number used changes over time) for branches containing code for older major WordPress releases. Using LTS sounds great in overall, but the change happens once per year, so maybe it's fine to apply the change explicitly with a PR like this one. |
@@ -5,7 +5,7 @@ The following guide is for setting up your local environment to contribute to th | |||
## Prerequisites | |||
|
|||
- Node.js | |||
Gutenberg is a JavaScript project and requires [Node.js](https://nodejs.org/). The project is built using the latest active LTS release of node, and the latest version of NPM. See the [LTS release schedule](https://github.com/nodejs/Release#release-schedule) for details. | |||
Gutenberg is a JavaScript project and requires [Node.js](https://nodejs.org/). The project is built using the version (14.18.1) release of node, and the latest version of NPM. See the [LTS release schedule](https://github.com/nodejs/Release#release-schedule) for details. |
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.
At the moment we also use npm v6 rather than the latest one.
@@ -21,10 +21,10 @@ curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.38.0/install.sh | bash | |||
``` | |||
|
|||
Quit and restart terminal | |||
Install the long-term support (lts) version of node. | |||
Install node version (14.18.1). |
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.
I think we can keep this paragraph as is. See https://nodejs.org/en/about/releases/. Node 14 is considered Maintenance LTS release.
@gziolo What do you think of latest changes? I updated things so I am more generically using v14. I am not sure about keeping that one LTS line the same as I feel it may be a bit confusing. Thoughts? |
@mkaz and @ryanwelcher, would you mind helping polishing the documentation changes? 😄 The part where we use 14 as the Node.js version is solid 👍🏻 |
…pdate/node-version-info
Thanks @gziolo . Also, managed to get the checks to pass again. 👍 |
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.
Let's use consistently v6 and v14 for versions based on the Node.js releases page:
docs/contributors/code/getting-started-with-code-contribution.md
Outdated
Show resolved
Hide resolved
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.
@alexstine, thank you for wrangling these changes, much appreciated 🙇🏻
@noisysocks, you might want to cherry-pick this PR to the 5.9 release. It's going to be necessary at some point when we create |
I have just tested with the latest |
…36744) * Documentation updates and better NVM support. * Be more generic about version 14. * Merge trunk in to branch. * Apply suggestions from code review Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
Description
Instead of using Node.js lts version in .nvmrc and other documentation, this PR switches items to use version 14.18.1. I've found that the lts version can vary from system to system and this has gotten me a couple times. It seems hardcoding the correct version is the appropriate option to fix any confusion going forward.
Why the Change?
This is what happens when running the nvm install command as mentioned in documentation.
`alexstine@asm gutenberg % nvm install --lts
Installing latest LTS version.
v16.13.0 is already installed.
Now using node v16.13.0 (npm v8.1.0)`
I'm not sure exactly why this started happening, but these versions do not work with Gutenberg.
`alexstine@asm gutenberg % node --version
v16.13.0
alexstine@asm gutenberg % npm --version
8.1.0`
While I am not in favor of using older versions, using what works is a must. After asking @tellthemachines on Slack, I decided that a PR with the correct version instead of a generic "lts/*" may be an improvement.
I kind of wonder if this is just some strange issue with my system and this doesn't effect others. In that case, this can be closed out.
How has this been tested?
Whenever I run nvm use, the correct version is loaded up and things just work. 👍
Screenshots
Types of changes
Bug fix.
Checklist:
*.native.js
files for terms that need renaming or removal).