search

kelly / node-i2c

Node.js native bindings for i2c-dev. Plays well with Raspberry Pi and Beaglebone.
Other
216 stars 91 forks source link

Update i2c.cc for node v4.0.0+ compatibility #72

Closed jnovack closed 8 years ago

jnovack commented 8 years ago

Buffer::New() constructor changed moving into node v4.

This is the quickest and dirtiest way to make it compatible with all.

NOW it's compatible with node > 0.12.0 :)

Updating patch version to comply with semver rules.

bradjc commented 8 years ago

Compiles for me.

$ node --version
v5.3.0
jnovack commented 8 years ago

Any thoughts, comments, suggestions?

kelly commented 8 years ago

I mentioned this in another pr, but i'm looking for a solution that uses NaN https://www.npmjs.com/package/nan. Otherwise these problems are going to keep coming up.

jnovack commented 8 years ago

Yes, but in the meantime, how about a quick official fix so everyone does not have to download and compile their own version,

Secondly, why not just continuely update rather than support 0.8x for eternity. We're up to 5.0. it's time to upgrade. people can choose to grab old packages if they need old support.

kelly commented 8 years ago

At this point I don't feel comfortable accepting a temporary fix. Many people running embedded computers are running older versions of node.

jnovack commented 8 years ago

I'm not trying to argue here, but that's LITERALLY what "+#if( NODE_MAJOR_VERSION >= 4 )" is for.To support those running older versions. Addition, in package.json, the engines stanza prevents older versions from upgrading.

There are safety measures in place, we should take advantage of them.

kelly commented 8 years ago

I get that, but like I initially said, it's a hack and doesn't protect against future issues. Things will likely change again, and I don't want to add additional version matching logic. All of this is safely dealt with using NaN.