Skip to content

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Oct 23, 2016

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

Remove an unused function.

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

/cc @nodejs/v8-inspector

@mscdex mscdex added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Oct 23, 2016
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 23, 2016
@bnoordhuis
Copy link
Member

See #9220 for an alternative take. Whether it's unused or not depends on whether the C++ standard library exports a std::to_string function.

@mscdex
Copy link
Contributor Author

mscdex commented Oct 23, 2016

@bnoordhuis I can't seem to find what actually uses to_string() (whether it's built-in or not)? I can't find any references in src/, test/, or deps/v8_inspector/.

@bnoordhuis
Copy link
Member

Oh, it might be vestigial from the initial version of #8919. If it's not actually in use (and the CI seems to confirm that), then it LGTM.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Oct 25, 2016
PR-URL: #9243
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@jasnell
Copy link
Member

jasnell commented Oct 25, 2016

Landed in ed41d8d

@jasnell jasnell closed this Oct 25, 2016
@mscdex mscdex deleted the inspector-remove-unused-func branch October 25, 2016 22:19
evanlucas pushed a commit that referenced this pull request Nov 3, 2016
PR-URL: #9243
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
PR-URL: #9243
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 19, 2016
PR-URL: #9243
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
PR-URL: #9243
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
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