Skip to content
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

Document limitations of the ABI stability offered by N-API #332

Closed
gabrielschulhof opened this issue Aug 24, 2018 · 12 comments
Closed

Document limitations of the ABI stability offered by N-API #332

gabrielschulhof opened this issue Aug 24, 2018 · 12 comments
Milestone

Comments

@gabrielschulhof
Copy link
Collaborator

I opened a PR with some documentation modifications to remind that the ABI stability offered by N-API goes away if add-ons link to other Node.js APIs and/or external non-ABI-stable libraries.

AFAICT the two doc files touched by the PR are the only ones where it makes sense to mention this.

@gabrielschulhof gabrielschulhof added this to the Milestone 11 milestone Aug 24, 2018
@gabrielschulhof
Copy link
Collaborator Author

That PR has now landed. It touches doc/api/addons.md and doc/api/n-api.md. Perhaps we should add a similar blurb to node-addon-api's documentation.

@NickNaso
Copy link
Member

NickNaso commented Sep 4, 2018

@gabrielschulhof I think that we can write a brief recap on README and then link to a specific section where we would deeply explain what means ABI stability.

@gabrielschulhof
Copy link
Collaborator Author

@NickNaso I must've read your mind 🙂 nodejs/node-addon-api#326

@gabrielschulhof
Copy link
Collaborator Author

@NickNaso I don't have the link to the deep explanation yet, but maybe that belongs into the main repo and we can link to it there.

@NickNaso
Copy link
Member

NickNaso commented Sep 4, 2018

@gabrielschulhof And if we create a file inside the doc folder (e.g abi_stability.md) and write all the necessary things about ABI stability there?

@gabrielschulhof
Copy link
Collaborator Author

@NickNaso I'd rather do it in the main repo, because ABI stability is offered by N-API, not node-addon-api, so I'd like to reuse the explanation for those using N-API directly.

@NickNaso
Copy link
Member

NickNaso commented Sep 4, 2018

@gabrielschulhof Ok so we can just link it from node-addon-api.

@mhdawson
Copy link
Member

PR landed on node-addon-api.

@mhdawson
Copy link
Member

Need to check if equivalent has landed in core @gabrielschulhof

@gabrielschulhof
Copy link
Collaborator Author

@mhdawson the equivalent has landed in core: nodejs/node#22508

This issue cannot be closed yet because in the above comments starting at this one @NickNaso and I talk about creating a document inside the Node.js main repo's documentation subdirectory that explains the issues of ABI stability in detail.

@mhdawson
Copy link
Member

mhdawson commented Oct 4, 2018

Outstanding issue was to created detailed info on, has created PR on core: nodejs/node#23229

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Oct 6, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: nodejs#23229
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Oct 6, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: #23229
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Oct 7, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: #23229
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Oct 9, 2018
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Oct 9, 2018
gabrielschulhof pushed a commit to nodejs/node that referenced this issue Oct 9, 2018
Provides a link from the N-API reference to the guide discussing ABI
stability in greater depth.

Re: nodejs/abi-stable-node#332
PR-URL: #23287
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Oct 10, 2018
Provides a link from the N-API reference to the guide discussing ABI
stability in greater depth.

Re: nodejs/abi-stable-node#332
PR-URL: #23287
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
gabrielschulhof pushed a commit to nodejs/node-addon-api that referenced this issue Oct 10, 2018
Re: nodejs/abi-stable-node#332
PR-URL: #367
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
jasnell pushed a commit to nodejs/node that referenced this issue Oct 17, 2018
Re: nodejs/abi-stable-node#332 (comment)
PR-URL: #23229
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
jasnell pushed a commit to nodejs/node that referenced this issue Oct 17, 2018
Provides a link from the N-API reference to the guide discussing ABI
stability in greater depth.

Re: nodejs/abi-stable-node#332
PR-URL: #23287
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
@mhdawson
Copy link
Member

Landed all updates.

kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this issue Aug 24, 2022
Re: nodejs/abi-stable-node#332
PR-URL: nodejs/node-addon-api#367
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this issue Aug 26, 2022
Re: nodejs/abi-stable-node#332
PR-URL: nodejs/node-addon-api#367
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this issue Sep 19, 2022
Re: nodejs/abi-stable-node#332
PR-URL: nodejs/node-addon-api#367
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this issue Aug 11, 2023
Re: nodejs/abi-stable-node#332
PR-URL: nodejs/node-addon-api#367
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants