Skip to content

doc: clarify use of NAPI_EXPERIMENTAL - #35195

Closed
mhdawson wants to merge 9 commits into
nodejs:masterfrom
mhdawson:napi-doc-verinfo
Closed

doc: clarify use of NAPI_EXPERIMENTAL#35195
mhdawson wants to merge 9 commits into
nodejs:masterfrom
mhdawson:napi-doc-verinfo

Conversation

@mhdawson

@mhdawsonmhdawson commented Sep 14, 2020•
edited
Loading

Copy link
Copy Markdown
Member

We've had a few questions about APIs not being available which
were related to not having specified NAPI_EXPERIMENTAL.

Add some additional documentation to explain this common
issue:

Refs: nodejs/node-addon-api#810

Signed-off-by: Michael Dawson mdawson@devrus.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@mhdawson
doc: clarify use of NAPI_EXPERIMENTAL
74c6564
We've had a few questions about APIs not being available which
were related to not having specified `NAPI_EXPERIMENTAL`.
Add some additional documentation to explain this common
issue:
Refs: nodejs/node-addon-api#810
Signed-off-by: Michael Dawson <mdawson@devrus.com>
@nodejs-github-bot

nodejs-github-bot commented Sep 14, 2020

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/n-api

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels Sep 14, 2020
NickNaso
NickNaso approved these changes Sep 15, 2020
View reviewed changes

@NickNasoNickNaso left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

gabrielschulhof
gabrielschulhof reviewed Sep 17, 2020
View reviewed changes

@gabrielschulhofgabrielschulhof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Per our guidelines we should avoid using "you" in our documentation. I tried to re-word it more passively.

Comment threaddoc/api/n-api.md Outdated
Comment threaddoc/api/n-api.md Outdated
Comment threaddoc/api/n-api.md Outdated
Comment threaddoc/api/n-api.md Outdated
mhdawsonand others added 4 commits September 17, 2020 14:37
@mhdawson@gabrielschulhof
Update doc/api/n-api.md
e1e0178
Co-authored-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
@mhdawson@gabrielschulhof
Update doc/api/n-api.md
36d8e94
Co-authored-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
@mhdawson@gabrielschulhof
Update doc/api/n-api.md
9a976c2
Co-authored-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
@mhdawson@gabrielschulhof
Update doc/api/n-api.md
5c5e4f6
Co-authored-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
mhdawson
mhdawson commented Sep 17, 2020
View reviewed changes
Comment threaddoc/api/n-api.md Outdated
@mhdawson

mhdawson commented Sep 17, 2020

Copy link
Copy Markdown
MemberAuthor

@gabrielschulhof updated

gabrielschulhof
gabrielschulhof approved these changes Sep 17, 2020
View reviewed changes

@gabrielschulhofgabrielschulhof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I realized that as it is the doc never explains that N-API version: and added in: are headers accompanying each N-API. I suggested improved wording to explain that.

Comment threaddoc/api/n-api.md Outdated
Comment threaddoc/api/n-api.md Outdated
mhdawsonand others added 3 commits September 18, 2020 16:08
@mhdawson@gabrielschulhof
Update doc/api/n-api.md
d8d8c90
Co-authored-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
@mhdawson@gabrielschulhof
Update doc/api/n-api.md
4ed6bcc
Co-authored-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
@mhdawson
fix linter issues
0162f80
@mhdawson

mhdawson commented Sep 18, 2020

Copy link
Copy Markdown
MemberAuthor

Linters look ok, doc only landing

mhdawson added a commit that referenced this pull request Sep 18, 2020
@mhdawson
doc: clarify use of NAPI_EXPERIMENTAL
f4586c9
We've had a few questions about APIs not being available which
were related to not having specified `NAPI_EXPERIMENTAL`.
Add some additional documentation to explain this common
issue:
Refs: nodejs/node-addon-api#810
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #35195
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@mhdawson

mhdawson commented Sep 18, 2020

Copy link
Copy Markdown
MemberAuthor

Landed in f4586c9

ruyadorno pushed a commit that referenced this pull request Sep 21, 2020
@mhdawson@ruyadorno
doc: clarify use of NAPI_EXPERIMENTAL
6119e95
We've had a few questions about APIs not being available which
were related to not having specified `NAPI_EXPERIMENTAL`.
Add some additional documentation to explain this common
issue:
Refs: nodejs/node-addon-api#810
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #35195
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@ruyadornoruyadorno mentioned this pull request Sep 21, 2020
Merged
4 tasks
addaleax pushed a commit that referenced this pull request Sep 22, 2020
@mhdawson@addaleax
doc: clarify use of NAPI_EXPERIMENTAL
6669eff
We've had a few questions about APIs not being available which
were related to not having specified `NAPI_EXPERIMENTAL`.
Add some additional documentation to explain this common
issue:
Refs: nodejs/node-addon-api#810
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #35195
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Merged
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@mhdawson@joesepi
doc: clarify use of NAPI_EXPERIMENTAL
d7b075e
We've had a few questions about APIs not being available which
were related to not having specified `NAPI_EXPERIMENTAL`.
Add some additional documentation to explain this common
issue:
Refs: nodejs/node-addon-api#810
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: nodejs#35195
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gabrielschulhofgabrielschulhofgabrielschulhof approved these changes
@gengjiawengengjiawengengjiawen approved these changes
@legendecaslegendecaslegendecas approved these changes
+1 more reviewer
@NickNasoNickNasoNickNaso approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docIssues and PRs related to the documentations.node-apiIssues and PRs related to the Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mhdawson@nodejs-github-bot@NickNaso@gabrielschulhof@gengjiawen@legendecas