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

doc: n-api: out params for TypedArray info can be NULL #40371

Closed
wants to merge 1 commit into from

Conversation

isaacbrodsky
Copy link
Contributor

@isaacbrodsky isaacbrodsky commented Oct 8, 2021

This is useful information to have for applications that don't need to read the other properties. The implementation checks for nullptr, see:

napi_status napi_get_typedarray_info(napi_env env,

@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 Oct 8, 2021
doc/api/n-api.md Outdated Show resolved Hide resolved
doc/api/n-api.md Outdated Show resolved Hide resolved
This is useful information to have for applications that don't need to read the other properties. The implementation checks for `nullptr`, see: https://github.com/nodejs/node/blob/master/src/js_native_api_v8.cc#L2879
@isaacbrodsky
Copy link
Contributor Author

@VoltrexMaster lint should be fixed now, thanks

@Trott
Copy link
Member

Trott commented Oct 8, 2021

For whoever lands this: The commit message needs only one colon and the first thing after the colon should be an imperative verb. Maybe this? doc: indicate n-api out parameters that may be NULL Or something like that.

@isaacbrodsky If you fell like saving someone a few keystrokes and making that change to the commit message on your patch-1 branch, that would be great. (But no worries if not.)

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

@lpinca
Copy link
Member

lpinca commented Oct 8, 2021

To add to what @Trott wrote, commit message body should be wrapped at 72 characters per line.

@Mesteery Mesteery added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 9, 2021
mhdawson pushed a commit that referenced this pull request Oct 13, 2021
This is useful information to have for applications that don't need to read the other properties. The implementation checks for `nullptr`, see: https://github.com/nodejs/node/blob/master/src/js_native_api_v8.cc#L2879

PR-URL: #40371
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson
Copy link
Member

Landed in 4cf5563

@mhdawson mhdawson closed this Oct 13, 2021
targos pushed a commit that referenced this pull request Nov 4, 2021
This is useful information to have for applications that don't need to read the other properties. The implementation checks for `nullptr`, see: https://github.com/nodejs/node/blob/master/src/js_native_api_v8.cc#L2879

PR-URL: #40371
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Nov 26, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.

None yet

10 participants