Skip to content

util: reconstruct constructor in more cases - #27668

Closed
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:reconstruct-constructor-inspect
Closed

util: reconstruct constructor in more cases#27668
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:reconstruct-constructor-inspect

Conversation

@BridgeAR

Copy link
Copy Markdown
Member

This makes sure the constructor is reconstructed in cases where we
otherwise would not be able to detect the actual constructor anymore.

That way some util.inspect output is improved.

// cc @nodejs/util

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-botnodejs-github-bot added the util Issues and PRs related to the built-in util module. label May 13, 2019
@BridgeAR
BridgeAR requested review from addaleax and antsmartian and removed request for addaleaxMay 14, 2019 12:00
@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 14, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@TrottTrott 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.

Bonus points if you run coverage to make sure all branches in the lib/internal/util/inspect.js changes are covered by existing/new tests.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

@Trott

Bonus points if you run coverage to make sure all branches in the lib/internal/util/inspect.js changes are covered by existing/new tests.

I did not run the coverage but there are tests that verify that all branches are covered. I am also about to open a follow-up PR that adds more tests, so this should not be an issue.

This makes sure the constructor is reconstructed in cases where we
otherwise would not be able to detect the actual constructor anymore.
That way some `util.inspect` output is improved.
PR-URL: nodejs#27668
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@BridgeAR
BridgeARforce-pushed the reconstruct-constructor-inspect branch from 7bd3963 to 1bef82dCompareMay 16, 2019 12:58
BridgeAR added a commit that referenced this pull request May 16, 2019
This makes sure the constructor is reconstructed in cases where we
otherwise would not be able to detect the actual constructor anymore.
That way some `util.inspect` output is improved.
PR-URL: #27668
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@BridgeAR

Copy link
Copy Markdown
MemberAuthor

Landed in a92ad36 🎉

targos pushed a commit that referenced this pull request May 17, 2019
This makes sure the constructor is reconstructed in cases where we
otherwise would not be able to detect the actual constructor anymore.
That way some `util.inspect` output is improved.
PR-URL: #27668
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@BridgeARBridgeAR mentioned this pull request May 21, 2019
4 tasks
@BridgeAR
BridgeAR deleted the reconstruct-constructor-inspect branch January 20, 2020 12:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.utilIssues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@BridgeAR@nodejs-github-bot@Trott@addaleax@antsmartian