FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: n-api: out params for TypedArray info can be NULL by isaacbrodsky · Pull Request #40371 · nodejs/node · GitHub

/ node Public

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

Closed
isaacbrodsky wants to merge 1 commit into
nodejs:masterfrom
isaacbrodsky:patch-1
Closed

doc: n-api: out params for TypedArray info can be NULL#40371
isaacbrodsky wants to merge 1 commit into
nodejs:masterfrom
isaacbrodsky:patch-1

Conversation

isaacbrodsky commented Oct 8, 2021
edited
Loading

Copy link
Copy Markdown
Contributor

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 added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels Oct 8, 2021
Comment thread doc/api/n-api.md Outdated
Comment thread doc/api/n-api.md Outdated
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

Copy link
Copy Markdown
Contributor Author

@VoltrexMaster lint should be fixed now, thanks

Trott commented Oct 8, 2021

Copy link
Copy Markdown
Member

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.)

mhdawson 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

lpinca commented Oct 8, 2021

Copy link
Copy Markdown
Member

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

Mesteery added the author ready PRs that have at least one approval, no outstanding review comments, 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>

Copy link
Copy Markdown
Member

Landed in 4cf5563

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 mentioned this pull request Nov 26, 2021
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, 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.

10 participants


Back | FazBrowse Home | New Git URL