Skip to content

Conversation

@isaacbrodsky
Copy link
Contributor

@isaacbrodskyisaacbrodsky 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-botnodejs-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
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
ContributorAuthor

@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

@mhdawsonmhdawson 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

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

@MesteeryMesteery 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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
@mhdawson
Copy link
Member

Landed in 4cf5563

@BethGriggsBethGriggs mentioned this pull request Oct 14, 2021
2 tasks
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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Nov 26, 2021
1 task
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.docIssues and PRs related to the documentations.node-apiIssues and PRs related to the Node-API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@isaacbrodsky@Trott@lpinca@mhdawson@jasnell@tniessen@legendecas@VoltrexKeyva@nodejs-github-bot@Mesteery