Skip to content

Conversation

@mhdawson
Copy link
Member

@mhdawson mhdawson commented Feb 2, 2018

We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

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

doc, n-api

We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. dont-land-on-v4.x node-api Issues and PRs related to the Node-API. labels Feb 2, 2018
doc/api/n-api.md Outdated
*Note*: Calling `napi_wrap()` a second time on an object that already has a
native instance associated with it by virtue of a previous call to
`napi_wrap()` will cause an error to be returned. If you wish to associate
`napi_wrap()` will cause an error to be returned. In order to associate
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Get rid of In order:

To associate another native instance with the given object...

Nit: Consider getting rid of *Note*: at the start of this paragraph.

Nit: Get rid of that already has a native instance associated with it by virtue of a previous call to `napi_wrap()`. The sentence already says a second time and adding all that wordy context just makes it confusing. Also s/cause an error to be returned/return an error/. Also s/given//. Also.... Aw heck, here's what I think this whole too-long paragraph should be reduced to:

Calling napi_wrap() a second time on an object will return an error. To associate
another native instance with the object, use napi_remove_wrap() first.

@mhdawson
Copy link
Member Author

mhdawson commented Feb 5, 2018

Pushed commit to address comments.

@mhdawson
Copy link
Member Author

mhdawson commented Feb 5, 2018

@mhdawson
Copy link
Member Author

mhdawson commented Feb 5, 2018

CI failures are unrelated. Opened #18584 for odroid failure.

Going to land.

@mhdawson
Copy link
Member Author

mhdawson commented Feb 5, 2018

Landed as 629a447

@mhdawson mhdawson closed this Feb 5, 2018
mhdawson added a commit that referenced this pull request Feb 5, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: #18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: #18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: #18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: #18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 12, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: nodejs#18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 16, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: nodejs#18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

Backport-PR-URL: #19447
PR-URL: #18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
MylesBorins pushed a commit that referenced this pull request May 1, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

Backport-PR-URL: #19265
PR-URL: #18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
We avoid using 'you' in the documentation based on our
guidelines. Remove usage in the n-api doc.

PR-URL: nodejs#18528
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@mhdawson mhdawson deleted the napi-doc-you branch September 30, 2019 13:15
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.

8 participants