doc: add reference to guide for N-API additions #22593
Conversation
Add reference to guide with requirements/principles for accepting additions to N-API.
COLLABORATOR_GUIDE.md
Outdated
| @@ -21,6 +21,7 @@ | |||
| - [When Breaking Changes Actually Break Things](#when-breaking-changes-actually-break-things) | |||
| - [Reverting commits](#reverting-commits) | |||
| - [Introducing New Modules](#introducing-new-modules) | |||
| - [Additions to N-API](#additions-to-N-API) | |||
There was a problem hiding this comment.
#additions-to-N-API -> #additions-to-n-api (otherwise the link does not work).
COLLABORATOR_GUIDE.md
Outdated
| This | ||
| [guide](https://github.com/nodejs/node/blob/master/doc/guides/adding-new-napi-api.md) | ||
| outlines the requirements and principals that we should follow when | ||
| approving and landing new N-API APIs (any additions to node_api.h and |
There was a problem hiding this comment.
node_api.h -> `node_api.h`
COLLABORATOR_GUIDE.md
Outdated
| [guide](https://github.com/nodejs/node/blob/master/doc/guides/adding-new-napi-api.md) | ||
| outlines the requirements and principals that we should follow when | ||
| approving and landing new N-API APIs (any additions to node_api.h and | ||
| node_api_types.h). |
There was a problem hiding this comment.
node_api_types.h -> `node_api_types.h`
vsemozhetbyt
added
the
author ready
COLLABORATOR_GUIDE.md
Outdated
|
|
||
| This | ||
| [guide](https://github.com/nodejs/node/blob/master/doc/guides/adding-new-napi-api.md) | ||
| outlines the requirements and principals that we should follow when |
There was a problem hiding this comment.
s/principals/principles/
|
@nodejs/tsc, @Trott since this imposes additional requirements on PR review, and there was some concern expressed it was not obvious when the PR to land the guide that this was the case. |
COLLABORATOR_GUIDE.md
Outdated
|
|
||
| Two of the key requirements are that additions be landed as | ||
| experimental and then only promoted out of experimental with agreement | ||
| from the [n-api team](https://github.com/orgs/nodejs/teams/n-api). |
There was a problem hiding this comment.
I'd prefer this paragraph be removed. Leaving it in means we have to update things in two places if anything changes. More importantly, if this paragraph has a subtle-but-meaningful difference from what's in the linked doc, it may be unclear as to which document carries more authority.
I understand the motivation to summarize, but if someone is adding an N-API addition, they should (and no doubt will, if they see this section) read the linked doc. If they aren't adding an N-API addition, then this paragraph is not useful to them.
vsemozhetbyt
removed
the
author ready
|
@Trott @vsemozhetbyt pushed changes to address comments. |
|
@Trott do you think this is ready to go or should we try to get additional TSC reviewers? |
|
@mhdawson Ship it! |
|
Landed as 5cccc55 |
Add reference to guide with requirements/principles for accepting additions to N-API. PR-URL: #22593 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add reference to guide with requirements/principles for accepting additions to N-API. PR-URL: #22593 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add reference to guide with requirements/principles for accepting additions to N-API. PR-URL: #22593 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add reference to guide with requirements/principles for accepting additions to N-API. PR-URL: #22593 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add reference to guide with requirements/principles
for accepting additions to N-API.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes