-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Basic inspector support #2991
Basic inspector support #2991
Conversation
ea63c6b
to
049eca4
Compare
ff413b6
to
df38691
Compare
df38691
to
bf0861f
Compare
bf0861f
to
1dce877
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you @mtharrison - sorry this took so long for us to get landed!
🎉 this is awesome! I tested it with these two commands:
This one works, brilliantly. But I can't get this one to work:
As soon as I click "Inspect" in devtools I get above error. |
@bartlomieju |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bert discovered a problem - there's a debugger thread that's running unconditionally (even when --debug isn't running). This is causing a little failure in tests/workers_startup_bench.ts but it's indicative of a bigger design problem. Fix isn't super easy - so this will take a bit more to land - and I should have been reviewing more carefully : )
I am not sure if this is applicable here, but there were similar concerns
about a thread in Node - and we ended up having that “always on” thread
anyways. In the Node that thread is handling a signal used to enable
Inspector on a running instance if the Inspector was not enabled from the
command line.
…On Fri, Oct 4, 2019 at 5:56 PM Ryan Dahl ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Bert discovered a problem - there's a debugger thread that's running
unconditionally (even when --debug isn't running). This is causing a little
failure in tests/workers_startup_bench.ts but it's indicative of a bigger
design problem. Fix isn't super easy - so this will take a bit more to land
- and I should have been reviewing more carefully : )
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2991?email_source=notifications&email_token=AACGJLML7O3BDI3JLBG4DZLQM7Q4VA5CNFSM4IYRSU4KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCG7Y5AI#pullrequestreview-297766529>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACGJLP7YJRFYMUHZ65NO5DQM7Q4VANCNFSM4IYRSU4A>
.
|
Closing this because we are unable to get it green. We will try again soon. |
Continuation of #2696.