Skip to content
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

Updates for Node v12 #1743

Merged
merged 4 commits into from Dec 16, 2018
Merged

Updates for Node v12 #1743

merged 4 commits into from Dec 16, 2018

Conversation

zbjornson
Copy link
Contributor

I think this is everything.

The deprecation warnings about v8::StringObject::New are pending a new release of Nan and there's nothing to do in this module itself (nodejs/nan@509859c#commitcomment-31470115).

(BTW you have a ton of C4312, C4302 and C4311 conversion warnings with MSVC. I can fix those or at least silence the warning if you want in another PR.)

Fixes #1742

Copy link
Member

@reconbot reconbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is everything I always wanted ♥️

I'll fire it with some actual hardware tonight to make sure it's good but it sure looks good.

@reconbot
Copy link
Member

Fixing the warning for Windows is also welcome, I know we do some probably safe casting of ints to file handles (or the other way around?) but if it can be safer I'm for it.

@reconbot
Copy link
Member

@HipsterBrown if you're available tonight to help test I can work with you

@reconbot reconbot merged commit 1eecd60 into serialport:master Dec 16, 2018
@reconbot
Copy link
Member

looks good 🎉

@lock lock bot locked as resolved and limited conversation to collaborators Jun 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility with Node 12/master
2 participants