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

Revert "src: implement query callbacks for vm" #22911

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

This reverts commit 85c356c from PR #22390.

See the discussion in the (proposed) fix at #22836.

Fixes: #22723


I’ll update the fix PR with a revert of the revert as soon as this lands, so that we have an open PR where we can iron out the remaining issues here.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

This reverts commit 85c356c
from PR nodejs#22390.

See the discussion in the (proposed) fix at
nodejs#22836.

Refs: nodejs#22836
Refs: nodejs#22390
Fixes: nodejs#22723
@addaleax addaleax requested a review from jdalton September 17, 2018 18:18
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. vm Issues and PRs related to the vm subsystem. labels Sep 17, 2018
@addaleax
Copy link
Member Author

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

Please 👍 this comment to approve fast-tracking.

@addaleax addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 17, 2018
@addaleax
Copy link
Member Author

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 18, 2018
@addaleax
Copy link
Member Author

Windows re-build has (unrelated) failures, again: https://ci.nodejs.org/job/node-test-commit-windows-fanned/20818/

@addaleax
Copy link
Member Author

Landed in 8989c76

@addaleax addaleax closed this Sep 18, 2018
@addaleax addaleax deleted the revert-22390 branch September 18, 2018 13:49
addaleax added a commit that referenced this pull request Sep 18, 2018
This reverts commit 85c356c
from PR #22390.

See the discussion in the (proposed) fix at
#22836.

Refs: #22836
Refs: #22390
Fixes: #22723

PR-URL: #22911
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
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 pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Abort during Object.keys after vm.runInContext
8 participants