chore: pin node version #338
Conversation
package.json
Outdated
| }, | ||
| "engines": { | ||
| "node": "18.x", | ||
| "pnpm": ">=8.15.7 <9" |
There was a problem hiding this comment.
No need to pin PNPM version right?
There was a problem hiding this comment.
Yes @pavinduLakshan , PNPM seemed to be pinned in the release workflow , that's why I added it here, however we can remove it since the other workflows seem to be using the latest version.
There was a problem hiding this comment.
If there is a valid reason we need to pin the pnpm version of the workflow as well. but it's not necessary here imo.. @adibmbrk let's remove pinning the pnpm version
There was a problem hiding this comment.
What is the original problem we are trying to solve here.
Ideally, there should be an issue associated with the change.
@adibmbrk Shall we create one.
There was a problem hiding this comment.
IMO, we should not pin versions here and there, if it is pinned in the workflow itself, then we shouldn't use the engines and vise-versa.
There was a problem hiding this comment.
IMO, we should not pin versions here and there, if it is pinned in the workflow itself, then we shouldn't use the
enginesand vise-versa.
@brionmario IMO pinning the version in the workflow only corrects our CI, however developers setting up the repo locally will still face incompatible node-sass module due to incorrect node version, adding engines fixes that
|
37ba77f please update the commit message of this and follow the commit message guideline |
adibmbrk
force-pushed
the
pin-version
branch
from
37ba77f to
c90b841
Compare
Purpose
This PR pins the node version making it easier for developers to setup the oxygen ui project.
Related Issues
node-sassincompatibility with Node.jsv22.x#304Related PRs
#1or (None)Checklist
Security checks