Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Commit

Permalink
Build: Uses modules version instead of v8.
Browse files Browse the repository at this point in the history 
* Pointed out by @saper via #694.
PR URL: #743.
  • Loading branch information
@am11
am11 committed Mar 9, 2015
1 parent a73568f commit c5b4fa6
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions lib/extensions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,9 @@ function getBinaryName() {
} else if (process.env.SASS_BINARY_NAME) {
binaryName = process.env.SASS_BINARY_NAME;
} else {
var v8SemVersion = process.versions.v8.split('.').slice(0, 3).join('.');

binaryName = [process.platform, '-',
process.arch, '-',
v8SemVersion].join('');
process.versions.modules].join('');
}

return [binaryName, 'binding.node'].join('_');
Expand Down

6 comments on commit c5b4fa6

@saper
Copy link
Member

@saper saper commented on c5b4fa6 Mar 9, 2015

Choose a reason for hiding this comment

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

Very good! Thanks! Theoretically we should distinguish between iojs and node numbers, but right now node is at 14 and iojs jumped straight to 42 to follow with 43. Hopefully node will not catch up that quick :)

@am11
Copy link
Contributor Author

@am11 am11 commented on c5b4fa6 Mar 9, 2015

Choose a reason for hiding this comment

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

@saper, you mean like this: rvagg/archived-pangyp#1? Number approach in my opinion is the most ugly way of distinguishing runtime (which pangyp uses all over the place).
The ultimate solution will come from nodejs/node#493.

@saper
Copy link
Member

@saper saper commented on c5b4fa6 Mar 9, 2015

Choose a reason for hiding this comment

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

process.release looks very messy. Why can't we have process.config.variabes.node_name or something like this....

What is the number approach you dislike? (didn't understand)

@am11
Copy link
Contributor Author

@am11 am11 commented on c5b4fa6 Mar 9, 2015

Choose a reason for hiding this comment

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

You can propose them with this approach, but it looks lengthy process.config.variabes.node_name (too nested IMO).

What is the number approach you dislike? (didn't understand)

When node.js will move to v1, it will break all the packages relying on version number.

@saper
Copy link
Member

@saper saper commented on c5b4fa6 Mar 9, 2015

Choose a reason for hiding this comment

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

Ah, you are right of course. I need to learn a bit more a bit gyp/node-gyp/pangyp to contribute.

Do I understand it correctly: when http://github.com/TooTallNate/node-gyp accepts nodejs/node-gyp#564 there is no longer need for http://github.com/rvagg/pangyp ?

@am11
Copy link
Contributor Author

@am11 am11 commented on c5b4fa6 Mar 9, 2015

Choose a reason for hiding this comment

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

Yes pangyp is just a transient package. You can actually propose any idea to io.js even without learning that stuff. There are many enthusiasts who might be willing to implement it. :)

Please sign in to comment.