test-console: streamline arrow fn and refine inspect regex#11039
test-console: streamline arrow fn and refine inspect regex#11039jonniedarko wants to merge 1 commit intonodejs:masterfrom jonniedarko:test-console-cleanup
Conversation
removed unneccessary curly braces and return statement from inspect arrow function updated `assert.throws` regex to look for exact match at start of string
nodejs-github-bot
added
test
|
I personally find the removal of the curly braces more difficult to read, so I'm -1 on that part, but +1 on the other change. |
The way it is in this PR (no braces around an arrow function body if they are not required) is definitely the more common approach in our code base by a large margin: 474 vs 22. So I'm +1 on the change here (and making it 475 vs 21) in the hopes of one day getting us to a consistent style. (For the record, I don't have a strong personal preference for either style. I just want to stop seeing comments in both directions on pull requests. I want us to pick one, enforce it in the linter, and be done with it. :-D ) (EDIT: I do have a mild preference for explicit braces, but a much stronger preference for consistency.) |
|
I am not sure why the |
|
@jonniedarko don't worry about that, CI is 100% green. |
|
👍 |
removed unneccessary curly braces and return statement from inspect arrow function updated `assert.throws` regex to look for exact match at start of string PR-URL: #11039 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in ca3d131 |
|
@jasnell landed with wrong subsystem in commit title? Maybe there is still time to fix it. |
|
@lpinca Nah, 10 min window has passed :( Edit: Just realized you commented well within the time-limit. Sorry :D |
|
@jasnell so I know for future, should it of been |
|
@jonniedarko You are correct! |
removed unneccessary curly braces and return statement from inspect arrow function updated `assert.throws` regex to look for exact match at start of string PR-URL: #11039 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
removed unneccessary curly braces and return statement from inspect arrow function updated `assert.throws` regex to look for exact match at start of string PR-URL: #11039 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
removed unneccessary curly braces and return statement from inspect arrow function updated `assert.throws` regex to look for exact match at start of string PR-URL: #11039 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
9 participants
removed unnecessary curly braces and return statement from the
inspectarrow functionupdated
assert.throwsregex to look for an exact match at the start of the stringChecklist
make -j4 test(UNIX), orvcbuild test(Windows) passes