Copilot commented on code in PR #13882:
URL: https://github.com/apache/cloudstack/pull/13882#discussion_r4513532853
##########
ui/package.json:
##########
@@ -194,6 +194,9 @@
"jest": {
"testEnvironment": "node"
},
+ "overrides": {
+ "node-gyp": "^9.4.1"
+ },
Review Comment:
Top-level `overrides` is npm-specific (npm v8.3+). If this repository
installs the UI with Yarn or pnpm, this override may be ignored (Yarn typically
uses `resolutions`, pnpm uses `pnpm.overrides`). Consider aligning the
mechanism with the package manager used in CI/dev for the UI to ensure the
override actually takes effect.
##########
ui/package.json:
##########
@@ -194,6 +194,9 @@
"jest": {
"testEnvironment": "node"
},
+ "overrides": {
+ "node-gyp": "^9.4.1"
Review Comment:
Using a caret range (`^9.4.1`) can change the resolved `node-gyp` version
over time, which can reintroduce build regressions unexpectedly. If the intent
is to stabilize builds for newer Node versions, consider pinning to an exact
version (e.g., `9.4.1`) and ensuring the repo lockfile is updated accordingly
so CI/install behavior is deterministic.
##########
ui/package.json:
##########
@@ -194,6 +194,9 @@
"jest": {
"testEnvironment": "node"
},
+ "overrides": {
+ "node-gyp": "^9.4.1"
+ },
Review Comment:
The PR description says it 'upgrades deep dependencies' broadly, but the
diff only adds a single `node-gyp` override. Please either (a) adjust the
description to reflect the actual scope (pinning/overriding `node-gyp`), or (b)
include the additional dependency upgrades mentioned in the description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
