tools,test: make argument linting more stringent - #6720
Conversation
Sorry, something went wrong.
Sorry, something went wrong.
|
No accompaning rule change? Seems a bit pointless if you don't enforce the style. Note that I'm not really a fan of the alignment at (, but if it's the dominant style right now, so be it. |
Sorry, something went wrong.
|
@silverwind There is a change to tools/eslint-rules/align-function-arguments.js in this PR. |
Sorry, something went wrong.
|
Ah, totally missed that. LGTM. |
Sorry, something went wrong.
There was a problem hiding this comment.
Nittiest of nitpicks but if you leave the comma, the diff is one line shorter.
Sorry, something went wrong.
There was a problem hiding this comment.
I'm a supporter of the final-line dangling-comma but don't see it too much in the code base here. Happy to add an instance! And...
...done!
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
The custom linting rule for argument alignment in multi-line function calls previously ignored template strings in an effort to avoid false positives. This isn't really necessary. Enforce for template strings and adjust whitespace in three tests to abide. (Insert "The test abides" joke of your choosing here.)
|
Only CI failure is an unrelated known-flaky (with a possible fix pending in another PR, woot). |
Sorry, something went wrong.
The custom linting rule for argument alignment in multi-line function calls previously ignored template strings in an effort to avoid false positives. This isn't really necessary. Enforce for template strings and adjust whitespace in three tests to abide. (Insert "The test abides" joke of your choosing here.) PR-URL: nodejs#6720 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The custom linting rule for argument alignment in multi-line function calls previously ignored template strings in an effort to avoid false positives. This isn't really necessary. Enforce for template strings and adjust whitespace in three tests to abide. (Insert "The test abides" joke of your choosing here.) PR-URL: #6720 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Checklist
Affected core subsystem(s)
test tools
Description of change
The custom linting rule for argument alignment in multi-line function
calls previously ignored template strings in an effort to avoid false
positives. This isn't really necessary. Enforce for template strings and
adjust whitespace in three tests to abide. (Insert "The test abides"
joke of your choosing here.)