benchmark,lib,test: use braces for multiline block - #13828
Conversation
Sorry, something went wrong.
For if/else and loops where the bodies span more than one line, use curly braces. Refs: nodejs#13623 (comment)
|
|
||
| for (var k = 0; k < n; k++) | ||
| for (var i = 0; i < buffer.length; i++) | ||
| for (var k = 0; k < n; k++) { |
There was a problem hiding this comment.
I get why braces would be added here, but why also on the for below if it does not contain a multi-line body? Ditto for all other instances of this in this PR.
Sorry, something went wrong.
There was a problem hiding this comment.
@mscdex To make the decision about things like that, I looked at surrounding code and also used my own judgment as to what was more readable.
In this case, having one for with braces wrapping another for without braces seemed less readable to me than both having braces. Additionally, there were no other examples in this file of blocks without braces.
Sorry, something went wrong.
Sorry, something went wrong.
|
After such changes I usually ask: "Can you now turn on an ESLint rule?" |
Sorry, something went wrong.
For if/else and loops where the bodies span more than one line, use curly braces. PR-URL: nodejs#13828 Ref: nodejs#13623 (comment) Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
@Trott This doesn’t land cleanly on 8.x; if you can backport it, great, but if you don’t think it’s worth it feel free to add the dont-land label. |
Sorry, something went wrong.
For if/else and loops where the bodies span more than one line, use curly braces. PR-URL: nodejs#13828 Ref: nodejs#13623 (comment) Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Sorry, something went wrong.
For if/else and loops where the bodies span more than one line, use curly braces. Original-PR-URL: #13828 Ref: #13623 (comment) Original-Reviewed-By: Anna Henningsen <anna@addaleax.net> Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Original-Reviewed-By: Michaël Zasso <targos@protonmail.com> Original-Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #13995 Reviewed-By: Anna Henningsen <anna@addaleax.net>
For if/else and loops where the bodies span more than one line, use curly braces. Original-PR-URL: #13828 Ref: #13623 (comment) Original-Reviewed-By: Anna Henningsen <anna@addaleax.net> Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Original-Reviewed-By: Michaël Zasso <targos@protonmail.com> Original-Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #13995 Reviewed-By: Anna Henningsen <anna@addaleax.net>
For if/else and loops where the bodies span more than one line, use curly braces. Original-PR-URL: #13828 Ref: #13623 (comment) Original-Reviewed-By: Anna Henningsen <anna@addaleax.net> Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Original-Reviewed-By: Michaël Zasso <targos@protonmail.com> Original-Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #13995 Reviewed-By: Anna Henningsen <anna@addaleax.net>
For if/else and loops where the bodies span more than one line, use curly braces. Original-PR-URL: #13828 Ref: #13623 (comment) Original-Reviewed-By: Anna Henningsen <anna@addaleax.net> Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Original-Reviewed-By: Michaël Zasso <targos@protonmail.com> Original-Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #13995 Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label. |
Sorry, something went wrong.
For if/else and loops where the bodies span more than one line, use
curly braces.
Refs: #13623 (comment)
Checklist
Affected core subsystem(s)
benchmark lib test