Skip to content

Conversation

@eugeneo
Copy link
Contributor

@eugeneo eugeneo commented Dec 6, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

inspector: internal datastructures were slightly altered

Description of change

Inspector uses magical strings to communicate some events between
main thread and transport thread. This change replaces those strings
with enums that are more mainatainable (and remove unnecessary
encodings/decodings)

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol labels Dec 6, 2016
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with style nits.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space before '<' here and elsewhere.

@eugeneo
Copy link
Contributor Author

eugeneo commented Dec 9, 2016

Landed as d8c7534

@Fishrock123
Copy link
Contributor

@eugeneo Doesn't land cleanly on v7.x fwiw

@eugeneo
Copy link
Contributor Author

eugeneo commented Dec 13, 2016

italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
Inspector uses magical strings to communicate some events between
main thread and transport thread. This change replaces those strings
with enums that are more mainatainable (and remove unnecessary
encodings/decodings)

PR-URL: nodejs#10159
Reviewed-By: Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this pull request Jan 28, 2017
Inspector uses magical strings to communicate some events between
main thread and transport thread. This change replaces those strings
with enums that are more mainatainable (and remove unnecessary
encodings/decodings)

PR-URL: #10159
Reviewed-By: Ben Noordhuis <[email protected]>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
Inspector uses magical strings to communicate some events between
main thread and transport thread. This change replaces those strings
with enums that are more mainatainable (and remove unnecessary
encodings/decodings)

PR-URL: nodejs#10159
Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
Inspector uses magical strings to communicate some events between
main thread and transport thread. This change replaces those strings
with enums that are more mainatainable (and remove unnecessary
encodings/decodings)

PR-URL: nodejs#10159
Reviewed-By: Ben Noordhuis <[email protected]>
@jasnell
Copy link
Member

jasnell commented Mar 8, 2017

A backport PR would be required in order for this to land in v6

@eugeneo
Copy link
Contributor Author

eugeneo commented Mar 8, 2017

I don't think this needs to be backported...

@jasnell
Copy link
Member

jasnell commented Mar 8, 2017

Awesome. Thank you

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants