Support Atom shell's version of node and v8

[waTeim]

This fix is to allow the use of ContextDisposedNotification across incompatible declarations by using the atom-shell's NODE_VERSION_MINOR of 13 which is different from the currently used values of 10 and 11 by Joyent and so will not conflict with other software using contextify. Fixes #144

[kkoopa]

Thanks for reporting and submitting a patch. However, it would be better to also match newer versions, ensuring possible forward compatibility. Moreover, as this is a v8 compatibility issue, I'll update NAN to account for this change and then make use of the fix through that.

[waTeim]

No problem. As far as new versions go, who knows? But looks like updating nah.h is better if it can sense v8 versions independent of node versions.

[kkoopa]

It cannot directly sense v8 versions, but we are in the process of rolling out NAN 1.5, which will support io.js in addition to joyent/node. For this reason, we will have to add this and other stuff to NAN, accomodating new v8 versions and the fork of node.

I'll see to it that this works on both joyent/node and io.js, which should give support for new atom shell too.

On January 12, 2015 11:02:08 PM EET, Jeff Waller notifications@github.com wrote:

No problem. As far as new versions go, who knows? But looks like
updating nah.h is better if it can sense v8 versions independent of
node versions.

---

Reply to this email directly or view it on GitHub:
#145 (comment)

[kkoopa]

This will actually not be that easy. Checking for a minor version of 13 is not reliable.Checking for NODE_MODULE_VERSION >= 17 might work, but 17 is quite low to start with -- eventually joyent/node may itself run into module version 17. io.js went ahead and started at 42. If people are going to fork stuff and make incompatible changes they should add in some unique defines so it is at least possible to detect.

[waTeim]

Haha yea no kidding. But right now today there is a problem which has gotten worse as of yesterday because guess what version of v8 iojs uses? I agree that the simple check = 13 is not enough especially now (today as opposed to 2 days ago). Maybe NAN can deal with this and maybe not, but because people are going to start clamoring for something that will allow their stuff to work across lots of versions of v8 irrespective of the node version the check for node v.x ---> v8 v.y is not longer good enough, so how about today we try.

1. as a workaround find some signature define in v8.h 3.29 -- probably is one

[kkoopa]

Let's move this discussion to nodejs/nan#226. As you said, we need a reliable way of doing this. I have compiled a list of v8 changes that need addressing, ContextDisposedNotification is but one of them.

[waTeim]

Yep, did so, added a comment too. getting ahold of iojs locally currently.

[brianmcd]

I think this PR is obsolete now? Let me know if not.