-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: replace string concatenations with template literals #16046
Conversation
return cb(new Error('Inappropriate heading level\n' + | ||
JSON.stringify(tok))); | ||
return cb(new Error(`Inappropriate heading level | ||
${JSON.stringify(tok)}`)); |
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.
Nit - we actually prefer not to use multiline template strings. This should likely be changed while landing and we might think about adding a eslint rule against this as it comes up more often.
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.
The same applies to the other template strings in here.
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 besides the template strings.
The subsystem has to be changed to tools while landing. |
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.
Per @BridgeAR's suggestions
Landed in 9f98989 Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 ! |
PR-URL: #16046 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Lance Ball <[email protected]>
PR-URL: #16046 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Lance Ball <[email protected]>
PR-URL: nodejs/node#16046 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Lance Ball <[email protected]>
PR-URL: #16046 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Lance Ball <[email protected]>
PR-URL: #16046 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Lance Ball <[email protected]>
PR-URL: #16046 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Lance Ball <[email protected]>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
tooks