-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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.
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
errors: consistent format for error messages #16904
errors: consistent format for error messages #16904
Conversation
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I like literals better, but as long as there's one standard that's enforced by the linter, fine by me 😄
create: function(context) { | ||
return { | ||
ExpressionStatement: function(node) { | ||
if (!isDefiningError(node)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been meaning to do this, but while you're here, can you extract this to rules-utils.js
, and update the other usage of this?
@maclover7 Yeah, I'm fine with either. We currently have more printf-formatted errors so this made sense to me (and the fact that it avoids creating all these extra functions) but either is fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Landed in 4b82d89 |
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments. PR-URL: #16904 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Should this be backported to |
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments. PR-URL: nodejs#16904 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport in #17624 |
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments. Backport-PR-URL: #17624 PR-URL: #16904 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments. Backport-PR-URL: #17624 PR-URL: #16904 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments. Backport-PR-URL: #17624 PR-URL: #16904 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Should this be backported to v6.x and |
Consistently use printf-style strings for error messages that do not need a custom argument order or processing of arguments.
Also added an eslint rule (with a test) that checks for this and warns about it.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
errors, test, tools