Skip to content

tools: add docs for prefer-util-format-errors rule #17376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

tools: add docs for prefer-util-format-errors rule #17376

wants to merge 1 commit into from

Conversation

@maclover7
Copy link
Contributor

I had a little trouble understanding what the rule was trying to say, so
am documenting what would pass/fail.

Checklist
Affected core subsystem(s)

tools

@maclover7
tools: add docs for prefer-util-format-errors rule
4d5fb5b 
I had a little trouble understanding what the rule was trying to say, so
am documenting what would pass/fail.
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Nov 28, 2017
Trott
Trott approved these changes Nov 28, 2017
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM. One possible minor improvements if you feel so inclined: Show what the failing example could be rewritten as and (if it's not immediately obvious) an explanation of why it's better than the failing code.

gibfahn
gibfahn approved these changes Nov 29, 2017
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Very helpful, thanks!

mhdawson
mhdawson approved these changes Nov 29, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@apapirovski
Copy link
Contributor

Just an FYI, the tests for these lint rules can also often be used as supplementary information. For this rule: https://github.com/nodejs/node/blob/master/test/parallel/test-eslint-prefer-util-format-errors.js

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 30, 2017
@maclover7
Copy link
Contributor Author

CI looks to be passing: https://ci.nodejs.org/job/node-test-commit/14481/, landing...

@maclover7 maclover7 self-assigned this Nov 30, 2017
@maclover7
Copy link
Contributor Author

Landed in 85e34b0

@maclover7 maclover7 closed this Nov 30, 2017
@maclover7 maclover7 deleted the jm-doc-rule branch November 30, 2017 21:51
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 1, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@maclover7 @MylesBorins
tools: add docs for prefer-util-format-errors rule
a9d98e7 
I had a little trouble understanding what the rule was trying to say, so
am documenting what would pass/fail.

PR-URL: #17376
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@maclover7 @MylesBorins
tools: add docs for prefer-util-format-errors rule
e41344f 
I had a little trouble understanding what the rule was trying to say, so
am documenting what would pass/fail.

PR-URL: #17376
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
@gibfahn gibfahn mentioned this pull request Dec 19, 2017
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes

@cjihrig cjihrig cjihrig approved these changes

@mhdawson mhdawson mhdawson approved these changes

+2 more reviewers

@apapirovski apapirovski apapirovski approved these changes

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@maclover7 maclover7

Labels

tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@maclover7 @apapirovski @Trott @cjihrig @mhdawson @gibfahn @addaleax @nodejs-github-bot