-
Notifications
You must be signed in to change notification settings - Fork 501
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
Node 18 compatibility #937
Conversation
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.
Thanks for this - looks good!
Please also add Node 18 to Appveyor and TravisCI test matrices (although Travis is not currently being used) as was done in the following commit:
closes #936 |
@mkrufky done |
And, unfortunately the tests are failing to build:
They just need to be updated to match the new function signatures. |
@mkrufky There was a typo in the macro used to check the Node version |
Still seeing this error now, running Node 18, only. I suspect it's just a missing version on Appveyor's end, since 18 was only just released.
...and for some reason, I can't get Travis CI to run this repository, even within my own account. I'll re-run the appveyor tests tomorrow - maybe Appveyor will fix it on their own. I'll squash+merge this once it passes. |
This is the ticket that I filed on Appveyor: |
Any chance to get this into a release soon? |
Absolutely. Thank you for the reminder. I confess that I completely forgot about it. I will push a new version tonight or tomorrow.
…On May 24, 2022 10:55:49 AM GMT+03:00, "Gerhard Stöbich" ***@***.***> wrote:
Any chance to get this into a release soon?
|
It is done. |
NAN did have a small PR/release for Node 18 support. Might as well make that our minimum: nodejs/nan#937.
Compatibility with Node 18.0.0