search

electron / node-abi

:turtle: :rocket: Get the Node.js and Electron ABI for a given target and runtime
https://www.npmjs.com/node-abi
MIT License
164 stars 58 forks source link

Add all versions of node/electron & use semver #18

Closed fanatid closed 7 years ago

fanatid commented 7 years ago

Sorry for one big commit. Need major bump.

I changed getTarget to getTargets. Swap runtime and target/abi. Made arguments for functions mandatory (I think that clear is better than unclear).

node.json/electron.json can be updated with npm run update

fanatid commented 7 years ago

any feedback?

mafintosh commented 7 years ago

not a fan of the breaking api changes. i already have a bunch of upstream deps.

mafintosh commented 7 years ago

personally i like the simplicity of the module as is. this seems to up the complexity a bit, but that's @lgeiger's call.

lgeiger commented 7 years ago

@fanatid Sorry for not getting back to you and thanks for taking the time to contribute. The updating of the json list is quite interesting!

Though I totally agree with @mafintosh in that I'm not a fan of the breaking changes and like the simplicity of of the module as it is now.

We have a few upstream packages depending on node-abi which would all need to be modified. And this change doesn't add benefit to any of them.

What is your use case where you need all the available targets (Why getTarget to getTargets)?

fanatid commented 7 years ago

Thank you for feedback... I also don't like major changes. I'll try split this improvement for small parts. For beginning what you think about all versions node/electron in json files (and update script)?

fanatid commented 7 years ago

Moved to #21