Skip to content

Conversation

gabrielschulhof
Copy link
Contributor

Change the documentation for napi_define_class in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: @gabrielschulhof
Fixes: #36150

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

Change the documentation for `napi_define_class` in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: Gabriel Schulhof <[email protected]>
Fixes: nodejs#36150
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 18, 2020

Review requested:

  • @nodejs/n-api

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels Nov 18, 2020
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM with or without the suggestions I've left

gabrielschulhof pushed a commit that referenced this pull request Nov 20, 2020
Change the documentation for `napi_define_class` in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: Gabriel Schulhof <[email protected]>
Co-authored-by: Rich Trott <[email protected]>
Fixes: #36150
PR-URL: #36159
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@gabrielschulhof
Copy link
Contributor Author

Landed in bb3cbba.

@gabrielschulhof gabrielschulhof deleted the fix-napi-define-class-doc branch November 20, 2020 04:30
codebytere pushed a commit that referenced this pull request Nov 22, 2020
Change the documentation for `napi_define_class` in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: Gabriel Schulhof <[email protected]>
Co-authored-by: Rich Trott <[email protected]>
Fixes: #36150
PR-URL: #36159
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@codebytere codebytere mentioned this pull request Nov 22, 2020
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
Change the documentation for `napi_define_class` in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: Gabriel Schulhof <[email protected]>
Co-authored-by: Rich Trott <[email protected]>
Fixes: #36150
PR-URL: #36159
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
Change the documentation for `napi_define_class` in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: Gabriel Schulhof <[email protected]>
Co-authored-by: Rich Trott <[email protected]>
Fixes: #36150
PR-URL: #36159
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
Change the documentation for `napi_define_class` in such a way that
it mentions wrapping C++ class instances as a possible use for the API,
rather than making the assumption that it is the use case for the API.

Signed-off-by: Gabriel Schulhof <[email protected]>
Co-authored-by: Rich Trott <[email protected]>
Fixes: #36150
PR-URL: #36159
Reviewed-By: Rich Trott <[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
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify doc for napi_define_class to be less C++-specific
4 participants