Skip to content

util: show External values explicitly in inspect#12151

Closed
addaleax wants to merge 1 commit intonodejs:masterfrom
addaleax:inspect-external
Closed

util: show External values explicitly in inspect#12151
addaleax wants to merge 1 commit intonodejs:masterfrom
addaleax:inspect-external

Conversation

@addaleax
Copy link
Member

@addaleax addaleax commented Mar 31, 2017

Display v8::External values as [External] rather than {}
which makes them look like objects.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

util

@addaleax
util: show External values explicitly in inspect
87fcfc2 
Display `v8::External` values as `[External]` rather than `{}`
which makes them look like objects.
@addaleax addaleax added semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module. labels Mar 31, 2017
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module. labels Mar 31, 2017
@addaleax
Copy link
Member Author

addaleax commented Mar 31, 2017 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-commit/8814/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/682/

@addaleax
Copy link
Member Author

addaleax commented Apr 3, 2017

Landed in 3cc3e09

@addaleax addaleax closed this Apr 3, 2017
@addaleax addaleax deleted the inspect-external branch April 3, 2017 08:11
addaleax added a commit that referenced this pull request Apr 3, 2017
@addaleax
util: show External values explicitly in inspect
3cc3e09 
Display `v8::External` values as `[External]` rather than `{}`
which makes them look like objects.

PR-URL: #12151
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jasnell jasnell mentioned this pull request Apr 4, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

+1 more reviewer

@TimothyGu TimothyGu TimothyGu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@addaleax @bnoordhuis @jasnell @TimothyGu @cjihrig @nodejs-github-bot