-
Notifications
You must be signed in to change notification settings - Fork 1.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
test: fix addon test for Node.js 12 and V8 7.4 #1705
Conversation
V8 7.4 removes some API functions. Replace those with their NAN counterparts.
CI: https://ci.nodejs.org/view/All/job/nodegyp-test-pull-request/117/ (:heavy_check_mark:) |
CI with nightly: https://ci.nodejs.org/job/nodegyp-test-commit/573/ |
|
I had to shave it a little bit, but essentially since we put |
To actually pass in CITGM with Node.js master we'll need to publish this. |
Publishing a new version would indeed be great. CITGM is still relatively noisy and that would tune that noise down. |
I really want to get GYP3 into the next version... |
@refack any reason not to release two separate versions? I would really love to reduce the amount of noise with CITGM. It's hard to work with it otherwise. |
I'll try to get something out ASAP. |
@refack could we not just release 3.8.1 right quick? |
V8 7.4 removes some API functions. Replace those with their NAN counterparts. PR-URL: #1705 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
I've cherry picked this PR and #1534 over to |
We'd probably want #1713 as well. |
|
Checklist
npm install && npm test
passesDescription of change
V8 7.4 removes some API functions. Replace those with their NAN
counterparts.
Example current failure: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1781/nodes=debian9-64/testReport/junit/(root)/citgm/node_gyp_v3_8_0/