FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

errors,util: migrate to use internal/errors.js by bidipyne · Pull Request #13293 · nodejs/node · GitHub

/ node Public

errors,util: migrate to use internal/errors.js - #13293

Closed
bidipyne wants to merge 1 commit into
nodejs:masterfrom
bidipyne:my-branch
Closed

errors,util: migrate to use internal/errors.js#13293
bidipyne wants to merge 1 commit into
nodejs:masterfrom
bidipyne:my-branch

Conversation

Copy link
Copy Markdown
Contributor
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

util.js

ref: #11273

This is my first contribution in node. I read and understood the contribution guidelines, please review and suggest.
@jasnell
Thanks.

nodejs-github-bot added the util Issues and PRs related to the built-in util module. label May 30, 2017
targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label May 30, 2017
addaleax added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label May 30, 2017

jasnell commented Jun 1, 2017

Copy link
Copy Markdown
Member

jasnell commented Jun 2, 2017

Copy link
Copy Markdown
Member

@mhdawson @fhinkel ... could I ask one of you to please give this a review :-) thank you!

mhdawson 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM once --expose-internals is removed.

Comment thread test/parallel/test-util-inspect.js 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don' think we need --expose-internals

bidipyne commented Jun 2, 2017
edited
Loading

Copy link
Copy Markdown
Contributor Author

Done as per suggestion. Thanks! @mhdawson

mhdawson commented Jun 2, 2017

Copy link
Copy Markdown
Member

jasnell pushed a commit that referenced this pull request Jun 2, 2017
PR-URL: #13293
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

jasnell commented Jun 2, 2017

Copy link
Copy Markdown
Member

Landed in 1609899

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

errors Issues and PRs related to JavaScript errors originated in Node.js core. 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

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL