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

src: make implementing CancelPendingDelayedTasks for platform optional #30034

Closed

Conversation

@addaleax
Copy link
Member

addaleax commented Oct 19, 2019

Fold CancelPendingDelayedTasks() into UnregisterIsolate() and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Copy link
Member

bnoordhuis left a comment

LGTM. CI failure seems unrelated:

Error: ENOTEMPTY: directory not empty, rmdir 'c:\workspace\node-test-binary-windows-2\test\.tmp.125'

https://ci.nodejs.org/job/node-test-binary-windows-2/3621/COMPILED_BY=vs2017,RUNNER=win2008r2-vs2017,RUN_SUBSET=1/console

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Trott added a commit that referenced this pull request Oct 22, 2019
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

PR-URL: #30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@Trott

This comment has been minimized.

Copy link
Member

Trott commented Oct 22, 2019

Landed in 2dc7065

@Trott Trott closed this Oct 22, 2019
@addaleax addaleax deleted the addaleax:platform-no-cancel-pending-delayed branch Oct 22, 2019
MylesBorins added a commit that referenced this pull request Oct 23, 2019
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

PR-URL: #30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
MylesBorins added a commit that referenced this pull request Oct 23, 2019
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

PR-URL: #30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 23, 2019
targos added a commit that referenced this pull request Nov 8, 2019
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

PR-URL: #30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
targos added a commit that referenced this pull request Nov 10, 2019
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

PR-URL: #30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
targos added a commit that referenced this pull request Nov 11, 2019
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.

It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.

PR-URL: #30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.