Skip to content

inspector: stop relying on magic strings#10159

Merged
eugeneo merged 0 commit into
nodejs:masterfrom
eugeneo:better_queues
Dec 9, 2016
Merged

inspector: stop relying on magic strings#10159
eugeneo merged 0 commit into
nodejs:masterfrom
eugeneo:better_queues

Conversation

@eugeneo

@eugeneo eugeneo commented Dec 6, 2016

Copy link
Copy Markdown
Contributor
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

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with style nits.

Comment thread src/inspector_agent.cc Outdated

Copy link
Copy Markdown
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

eugeneo commented Dec 9, 2016

Copy link
Copy Markdown
Contributor Author

Landed as d8c7534

@Fishrock123

Copy link
Copy Markdown
Contributor

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

@eugeneo

eugeneo commented Dec 13, 2016

Copy link
Copy Markdown
Contributor Author

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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
@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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
@jasnell

jasnell commented Mar 8, 2017

Copy link
Copy Markdown
Member

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

@eugeneo

eugeneo commented Mar 8, 2017

Copy link
Copy Markdown
Contributor Author

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

@jasnell

jasnell commented Mar 8, 2017

Copy link
Copy Markdown
Member

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