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: clarify operation of napi_cancel_async_work #12974

Closed
wants to merge 3 commits into
base: master
from

Conversation

@mhdawson
Member

mhdawson commented May 11, 2017

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

Show outdated Hide outdated doc/api/n-api.md
@@ -2931,11 +2931,12 @@ NAPI_EXTERN napi_status napi_cancel_async_work(napi_env env,
Returns `napi_ok` if the API succeeded.
This API cancels a previously allocated work, provided
it has not yet been queued for execution. After this function is called
This API cancels a previously queued work if it has not yet

This comment has been minimized.

@Trott

Trott May 11, 2017

Member

Nit: a previously queued work? Should that just be previously queued work without the a?

Nit: previously queued -> previously-queued or (even better IMO) just queued

@Trott

Trott May 11, 2017

Member

Nit: a previously queued work? Should that just be previously queued work without the a?

Nit: previously queued -> previously-queued or (even better IMO) just queued

Show outdated Hide outdated doc/api/n-api.md
@@ -2931,11 +2931,12 @@ NAPI_EXTERN napi_status napi_cancel_async_work(napi_env env,
Returns `napi_ok` if the API succeeded.
This API cancels a previously allocated work, provided
it has not yet been queued for execution. After this function is called
This API cancels a previously queued work if it has not yet

This comment has been minimized.

@jasnell

jasnell May 11, 2017

Member

either drop 'a' or s/work/work item

@jasnell

jasnell May 11, 2017

Member

either drop 'a' or s/work/work item

Show outdated Hide outdated doc/api/n-api.md
the `complete` callback will be invoked with a status value of
`napi_cancelled`. The work should not be deleted before the `complete`
callback invocation, even when it was cancelled.
callback invocation, even when it was successfully cancelled.

This comment has been minimized.

@Trott

Trott May 11, 2017

Member

Nit: even when it was successfully cancelled -> even if it has been successfully cancelled?

@Trott

Trott May 11, 2017

Member

Nit: even when it was successfully cancelled -> even if it has been successfully cancelled?

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson May 12, 2017

Member

@Trott, @jasnell thanks for the comments. Commit pushed to address them.

Member

mhdawson commented May 12, 2017

@Trott, @jasnell thanks for the comments. Commit pushed to address them.

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson May 15, 2017

Member

Believe I have addressed @Trott 's comments and many approvals so going to land

CI Run: https://ci.nodejs.org/job/node-test-pull-request/8090/

Member

mhdawson commented May 15, 2017

Believe I have addressed @Trott 's comments and many approvals so going to land

CI Run: https://ci.nodejs.org/job/node-test-pull-request/8090/

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson May 16, 2017

Member

CI good, landing.

Member

mhdawson commented May 16, 2017

CI good, landing.

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson May 16, 2017

Member

Landed as: 1b28022

Member

mhdawson commented May 16, 2017

Landed as: 1b28022

@mhdawson mhdawson closed this May 16, 2017

mhdawson added a commit that referenced this pull request May 16, 2017

doc: clarify operation of napi_cancel_async_work
PR-URL: #12974
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

anchnk added a commit to anchnk/node that referenced this pull request May 19, 2017

doc: clarify operation of napi_cancel_async_work
PR-URL: nodejs#12974
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

@jasnell jasnell referenced this pull request May 28, 2017

Closed

8.0.0 Release Proposal #12220

@gibfahn gibfahn referenced this pull request Jun 15, 2017

Closed

Auditing for 6.11.1 #230

2 of 3 tasks complete

@mhdawson mhdawson deleted the mhdawson:napi-doc-update1 branch Jun 28, 2017

gabrielschulhof added a commit to gabrielschulhof/node that referenced this pull request Mar 12, 2018

doc: clarify operation of napi_cancel_async_work
PR-URL: nodejs#12974
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

gabrielschulhof added a commit to gabrielschulhof/node that referenced this pull request Mar 31, 2018

doc: clarify operation of napi_cancel_async_work
PR-URL: nodejs#12974
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

gabrielschulhof added a commit to gabrielschulhof/node that referenced this pull request Apr 10, 2018

doc: clarify operation of napi_cancel_async_work
PR-URL: nodejs#12974
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

MylesBorins added a commit that referenced this pull request Apr 16, 2018

doc: clarify operation of napi_cancel_async_work
Backport-PR-URL: #19447
PR-URL: #12974
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

@MylesBorins MylesBorins referenced this pull request Apr 16, 2018

Merged

v6.14.2 proposal #19996

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment