-
Notifications
You must be signed in to change notification settings - Fork 36
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
Cleans up the root package.json
#312
Conversation
Signed-off-by: Artem Zatsarynnyi <azatsary@redhat.com>
Pull Request Dev image published: |
@@ -10,13 +10,5 @@ | |||
"build:min": "cd code && yarn gulp vscode-reh-web-linux-x64-min", | |||
"rebuild-native-modules": "cd code && npm rebuild" | |||
}, | |||
"license": "EPL-2.0", | |||
"dependencies": { | |||
"node-gyp": "^9.4.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.
Removing node-gyp
from npm was a breaking change introduced in npm/cli#6932.
They already fixed the problem in npm/cli#6932.
While a new version of Node.js with the fixed npm has not yet been released, the only temporary workaround that we need on the Che-Code side is installing node-gyp
globally to allow any transitive dependency rely on it. Once we update to a newer Node.js, we can get rid of that ^^ workaround.
Anyway, if it was even decided that node-gyp
was no longer part of npm, we don't need to declare any transitive dependency of VS Code in Che-Code's root package.json.
"dependencies": { | ||
"node-gyp": "^9.4.1" | ||
}, | ||
"engines": { |
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.
VS Code build has a custom check of the node/yarn versions on the preinstall phase. I believe there's not much sense in introducing one more very similar verification. So, I suggest relying only on the verification script from upstream.
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.
moreover: it broke the che-in-che development: eclipse-che/che#22686
Pull Request Che-Code image published: |
Build 3.11 :: code_3.x/1017: Console, Changes, Git Data |
Build 3.11 :: sync-to-downstream_3.x/5362: Console, Changes, Git Data |
Build 3.11 :: push-latest-container-to-quay_3.x/3748: Console, Changes, Git Data |
Build 3.11 :: get-sources-rhpkg-container-build_3.x/5233: code : 3.x :: Build 57170751 : quay.io/devspaces/code-rhel8:3.11-34 |
Build 3.11 :: code_3.x/1017: Upstream sync done; /DS_CI/sync-to-downstream_3.x/5362 triggered |
Build 3.11 :: update-digests_3.x/4992: Console, Changes, Git Data |
Build 3.11 :: operator-bundle_3.x/2317: Console, Changes, Git Data |
Build 3.11 :: sync-to-downstream_3.x/5363: Console, Changes, Git Data |
Build 3.11 :: push-latest-container-to-quay_3.x/3750: Console, Changes, Git Data |
Build 3.11 :: copyIIBsToQuay/2191: Console, Changes, Git Data |
Build 3.11 :: sync-to-downstream_3.x/5363: Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/5235 triggered; /job/DS_CI/job/dsc_3.x triggered; |
Build 3.11 :: operator-bundle_3.x/2317: Upstream sync done; /DS_CI/sync-to-downstream_3.x/5363 triggered |
Build 3.11 :: dsc_3.x/1583: Console, Changes, Git Data |
Build 3.11 :: update-digests_3.x/4992: Detected new images: rebuild operator-bundle |
What does this PR do?
In this PR, I suggest removing the redundant fields from the root
package.json
that I overlooked while reviewing the recently merged patch. See the details in the inlined code comments below ⬇️What issues does this PR fix?
fixes eclipse-che/che#22686
How to test this PR?
Actually, the PR changes nothing except cleaning up the part of the root
package.json
that may confuse the developer.