-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
tools: sync gyp code base with node-gyp repo #30563
Conversation
PR in nodejs/node-gyp: nodejs/node-gyp#1975 |
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.
Hopefully I've seen everything, just a bunch of comments. Likely all the issues need to be picked up in the gyp3 repo instead, so feel free to ignore 'em and merge. Thanks for doing this!
Do we really want to get rid of tools/gyp/gyptest.py? Perhaps it should be renamed to tools/gyp/test_gyp.py so that pytest will discover it and modify it to run in our test cycles. |
@cclauss, that file was removed in nodejs/node-gyp#1458 |
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.
RSLGTM
@targos what's left to be addressed here? |
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.
LGTM once unblocked
Other PR landed on node-gyp. This should be ready to go. |
PR-URL: #30563 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
Landed in cf00961 |
PR-URL: #30563 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #30563 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #30563 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #30563 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#30563 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
This syncs the gyp code base so that it is identical to the one in nodejs/node-gyp.
This was done by removing the code in nodejs/node-gyp, copying from nodejs/node and then manually reviewing and adapting the diff.