Skip to content
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

New: function-call-argument-newline rule (fixes #10323) #12024

Merged
merged 1 commit into from Aug 18, 2019

Conversation

finico
Copy link
Contributor

@finico finico commented Jul 26, 2019

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[x] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What category of rule is this? (place an "X" next to just one item)

[x] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)

What changes did you make? (Give an overview)
All information about this rule in #10323

Is there anything you'd like reviewers to focus on?
I didn't add indentations in fixers. But I've checked other rules and they don't do it as well..

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jul 26, 2019
@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion feature This change adds a new feature to ESLint rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jul 27, 2019
@platinumazure platinumazure changed the title New: function-call-argument-newline rule (fixes: #10323) New: function-call-argument-newline rule (fixes #10323) Jul 27, 2019
Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @finico, thanks for the PR!

This looks good to me at a glance, but I would like to give other team members a chance to review. Thanks for contributing!

Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for contributing to ESLint!

@kaicataldo
Copy link
Member

Re indentation in fixers: since we have a multi-pass autofix system, we try to keep fixes to the minimum changes necessary and let other rules handle autofixing things they warn about. The indentation rule would fix this, so this is correct 👍.

@platinumazure platinumazure merged commit 8cd00b3 into eslint:master Aug 18, 2019
@platinumazure
Copy link
Member

Merged. Thanks for contributing! Sorry for the delay in getting this in.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants