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.

Sign up for GitHub

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

Jump to bottom

debugger: wait for V8 debugger to be enabled #38811

Closed
wants to merge 1 commit into from
Closed

debugger: wait for V8 debugger to be enabled #38811

wants to merge 1 commit into from

Conversation

targos
Copy link
Member

@targos targos commented May 26, 2021

@github-actions github-actions bot added inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. labels May 26, 2021
@targos
Copy link
Member Author

targos commented May 26, 2021

@nodejs/inspector

This is probably not entirely fixed (inspector.client.on('ready') still doesn't await the promise), but all tests pass with this PR and V8 9.1.

@targos targos mentioned this pull request May 26, 2021
Merged
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels May 26, 2021
@github-actions
Copy link
Contributor

Fast-track has been requested by @aduh95. Please 👍 to approve.

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 removed the fast-track PRs that do not need to wait for 48 hours to land. label May 28, 2021
@targos
Copy link
Member Author

targos commented May 28, 2021

CI is blocked on #38820

@nodejs-github-bot
Copy link
Collaborator

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

@targos targos mentioned this pull request May 28, 2021
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

targos added a commit that referenced this pull request May 28, 2021
@targos
debugger: wait for V8 debugger to be enabled
f1cbaea 
Refs: #38273 (comment)

PR-URL: #38811
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos
Copy link
Member Author

targos commented May 28, 2021

Landed in f1cbaea

@targos targos closed this May 28, 2021
@targos targos deleted the inspector-wait branch May 28, 2021 21:06
danielleadams pushed a commit that referenced this pull request May 31, 2021
@targos @danielleadams
debugger: wait for V8 debugger to be enabled
ee1056d 
Refs: #38273 (comment)

PR-URL: #38811
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request May 31, 2021
Merged
@Trott Trott mentioned this pull request Jul 7, 2021
Merged
@richardlau
Copy link
Member

Doesn't land cleanly on v14.x-staging.

@Trott
Copy link
Member

Trott commented Jul 16, 2021

Doesn't land cleanly on v14.x-staging.

It will land cleanly if you land #38411 and #38406 first, both of which will cherry-pick cleanly to v14.x-staging (at least if they are cherry-picked in that order).

I know you're skipping the primordials stuff generally for v14.x-staging, but maybe you would be OK making an exception for node inspect stuff only (which is what #38406 is)?

@Trott Trott mentioned this pull request Jul 16, 2021
Merged
aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 19, 2021
@targos @aduh95
debugger: wait for V8 debugger to be enabled
6ee4cbd 
Refs: nodejs#38273 (comment)

PR-URL: nodejs#38811
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@richardlau richardlau mentioned this pull request Jul 19, 2021
Closed
@richardlau
Copy link
Member

Backport to v14.x PR: #39446

aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 20, 2021
@targos @aduh95
debugger: wait for V8 debugger to be enabled
22a26bd 
Refs: nodejs#38273 (comment)

PR-URL: nodejs#38811
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 20, 2021
@targos @aduh95
debugger: wait for V8 debugger to be enabled
cc589f3 
Refs: nodejs#38273 (comment)

PR-URL: nodejs#38811
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
@targos @richardlau
debugger: wait for V8 debugger to be enabled
36050af 
Refs: #38273 (comment)

PR-URL: #38811
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
@targos @richardlau
debugger: wait for V8 debugger to be enabled
79bfb04 
Refs: #38273 (comment)

PR-URL: #38811
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@richardlau richardlau mentioned this pull request Jul 22, 2021
Merged
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@targos @foxxyz
debugger: wait for V8 debugger to be enabled
f9c6beb 
Refs: nodejs#38273 (comment)

PR-URL: nodejs#38811
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bl-ue bl-ue bl-ue approved these changes

@Flarna Flarna Flarna approved these changes

@aduh95 aduh95 aduh95 approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

@richardlau richardlau richardlau approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@targos @nodejs-github-bot @richardlau @Trott @BethGriggs @aduh95 @Flarna @bl-ue