-
Notifications
You must be signed in to change notification settings - Fork 45
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
Makes the lib compatible with Node 4.1.1 #18
Conversation
Thanks, and sorry for the delay in getting to this. I'm not excited about taking on the added step of babel compilation, just to support Node 4. Maybe I could be convinced though. I understand a substantial number of people still run Node 4. |
I think a better solution might be to just change the source code to be Node 4 compatible. I haven't tried, but I think it shouldn't be too hard:
|
…ode4-compatible
Sure, I can do this :). |
If this is done, then I can update noble/noble#646 to use this package with Node 4 also. |
now that node 4.x is EOL is this still worth doing? |
I'm pushing to make 6.x the minimum version for the next version of noble btw. |
@@ -514,7 +514,7 @@ NobleBindings.prototype._onAdvertisementWatcherReceived = function(sender, e) { | |||
var manufacturerData = manufacturerSections[0]; | |||
deviceRecord.manufacturerData = rt.toBuffer(manufacturerData.data); | |||
let companyIdHex = manufacturerData.companyId.toString(16); | |||
let toAppend = Buffer.allocUnsafe(2); | |||
let toAppend = new Buffer(2); |
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.
this will spew warnings in node 10.x and same for the other one.
I think not, therefore I'm going to go ahead and close this PR. If anyone objects, please speak up! |
Tested this on project that I'm working now and everything seems to be OK 👍