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.

Sign up for GitHub

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

Jump to bottom

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
Milestone 11

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.

@gabrielschulhof gabrielschulhof mentioned this issue Oct 2, 2018
Closed
3 tasks
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Oct 3, 2018
doc: add guide about abi stability
f9e2295 
Re: nodejs/abi-stable-node#332 (comment)
@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
doc: add guide about abi stability
65366ad 
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
@targos
doc: add guide about abi stability
9f264ef 
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
@targos
doc: add guide about abi stability
e5bfab0 
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
README.md: Link to new ABI stability guide
556d0c6 
Re: nodejs/abi-stable-node#332
@gabrielschulhof gabrielschulhof mentioned this issue Oct 9, 2018
Closed
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Oct 9, 2018
README.md: Link to new ABI stability guide
60effbf 
Re: nodejs/abi-stable-node#332
gabrielschulhof pushed a commit to nodejs/node that referenced this issue Oct 9, 2018
doc: add link to ABI guide
157d507 
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
@targos
doc: add link to ABI guide
5ff4300 
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
README.md: link to new ABI stability guide
fd65078 
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
@jasnell
doc: add guide about abi stability
86db67c 
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
@jasnell
doc: add link to ABI guide
df93034 
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
@kevindavies8
README.md: link to new ABI stability guide
3f54168 
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
@Marlyfleitas
README.md: link to new ABI stability guide
c0e695b 
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
@wroy7860
README.md: link to new ABI stability guide
6e19705 
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
README.md: link to new ABI stability guide
4ff6d50 
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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Milestone 11
Development

No branches or pull requests
3 participants
@NickNaso @gabrielschulhof @mhdawson