Docs: Update CONTRIBUTING.md to Include node and npm version requirement detail#540
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
CONTRIBUTING.md to Include nvm useCONTRIBUTING.md to Include nvm use
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #540 +/- ##
=============================================
+ Coverage 70.72% 71.20% +0.47%
- Complexity 1144 1150 +6
=============================================
Files 67 67
Lines 5510 5563 +53
=============================================
+ Hits 3897 3961 +64
+ Misses 1613 1602 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| ```bash | ||
| composer install && npm i && npm run build | ||
| composer install && nvm use && npm i && npm run build |
There was a problem hiding this comment.
Minor thing but I'll note there are other options to set the node and npm version outside of nvm. While I use nvm myself I realize not everyone does so not sure if we want to make that a requirement here.
I'd maybe suggest instead of adding nvm use to the step here, we just add a call out that those looking to contribute need to ensure they have the right node and npm version and maybe suggest nvm as a solution for that.
There was a problem hiding this comment.
Thanks @dkotter for the review,
I'll note there are other options to set the node and npm version outside of nvm
Yes Agree on this, while I mostly use nvm use as well.
I'd maybe suggest instead of adding nvm use to the step here, we just add a call out that those looking to contribute need to ensure they have the right node and npm version and maybe suggest nvm as a solution for that.
I could update the PR as per suggestion, to include that which refer .nvmrc for correct node version, and if using nvm to switch node versions, use nvm use before npm install step.
There was a problem hiding this comment.
Worth noting it's not just node version, it's npm version as well. These are documented in our package.json file so probably a good idea to update instructions to say something along the lines of verify your node and npm version match the minimum requirements in package.json
There was a problem hiding this comment.
Hi @dkotter, Have updated the messaging. Can you please review if anything needs to be updated/changed in the wordings, I would be happy to update that.
Thanks,
| @@ -1 +1 @@ | |||
| 22 | |||
| 22.21.1 | |||
There was a problem hiding this comment.
@dkotter, Have updated this to exact node version used in package.json. Reason being, when I initially cloned the repo, I was having node version 22.14.0, So nvm use pointed to that version. On build got error, because min required version is 22.21.1. Hence it should be good to have nvmrc file as the exact version.
Thanks,
There was a problem hiding this comment.
I guess my concern here is ideally people are using the latest compatible version of node. Right now the latest on the v22 branch is 22.22.3 but this will now force people to always use an older version. I think ideal is keeping this as 22 and if someone has an older version of v22 on their machine they'll need to update that for things to work.
There was a problem hiding this comment.
Makes sense, have not though of that. Will revert the change.
CONTRIBUTING.md to Include nvm useCONTRIBUTING.md to Include node and npm version requirement detail
What?
Closes #539
Why?
CONTRIBUTING.mdfile to include thenvm use, so developers/contributors can use the specified version of node from the repo.How?
nvm usecommand toInstall and build assetsstep.Use of AI Tools
Testing Instructions
Screenshots or screencast
Changelog Entry
Updated
CONTRIBUTING.mdto includenvm usecommand.