-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
tools: make argument alignment linting more strict #8642
Conversation
Hmmm, I remember this being a lot worse when we tried it before? (I think we tried it before?) |
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
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, and thank you!
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
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
LGTM. |
An upcoming custom lint rule will provide slightly more strict enforcement of argument alignment for multiline function calls. Adjust existing code to conform. PR-URL: nodejs#8642 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
A few nits in recent PR comments suggest that we can have slightly more strict linting for argument alignment in multiline function calls. This enables the existing linting requirements to apply when one or more of the arguments themselves are function calls. Previously, that situation had been excluded from linting. Refs: nodejs#8628 (comment) PR-URL: nodejs#8642 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@Trott I'm going to stop landing these changes to the linter in v4.x Please feel free to make a mass PR to get the two line in sync |
@thealphanerd I'm OK with the lint rules in v4.x being more lax than in master, so that's OK with me. If you'd prefer it land, I can try to backport. But if it's not high value to you, then I'm content to leave it out. |
I say leave it |
An upcoming custom lint rule will provide slightly more strict enforcement of argument alignment for multiline function calls. Adjust existing code to conform. PR-URL: #8642 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
A few nits in recent PR comments suggest that we can have slightly more strict linting for argument alignment in multiline function calls. This enables the existing linting requirements to apply when one or more of the arguments themselves are function calls. Previously, that situation had been excluded from linting. Refs: #8628 (comment) PR-URL: #8642 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
tools test lib
Description of change
A few nits in recent PR comments suggest that we can have slightly more
strict linting for argument alignment in multiline function calls. This
enables the existing linting requirements to apply when one or more of
the arguments themselves are function calls. Previously, that situation
had been excluded from linting.