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.

Sign up for GitHub

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

Jump to bottom

Breaking: no-confusing-arrow enable allowParens: true (fixes #11503) #11520

Merged
merged 3 commits into from
Apr 4, 2019
Merged

Breaking: no-confusing-arrow enable allowParens: true (fixes #11503) #11520

merged 3 commits into from
Apr 4, 2019

Conversation

aladdin-add
Copy link
Member

…es#11503)

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

[ x] Documentation update
[ x] Changes an existing rule ([template]

What changes did you make? (Give an overview)
fixes #11503

Is there anything you'd like reviewers to focus on?
no.

@aladdin-add aladdin-add added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible labels Mar 17, 2019
@aladdin-add aladdin-add changed the title Breaking: no-confusing-arrow enable allowParens: true by default (fix… Breaking: no-confusing-arrow enable allowParens: true (fixes #11503) Mar 17, 2019
platinumazure
platinumazure requested changes Mar 18, 2019
View reviewed changes
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.

I think there needs to be a unit test showing that the allowParens behavior is true by default. Could you please add a test for this case?

@not-an-aardvark not-an-aardvark added this to Implemented, pending review in v6.0.0 Mar 18, 2019
@aladdin-add aladdin-add force-pushed the issue11503 branch 2 times, most recently from d4f973f to 45f6acd Compare March 19, 2019 14:12
@aladdin-add
Copy link
Member Author

@platinumazure , good catch! I found the default value not applied, just pushed a commit to fix it.

platinumazure
platinumazure approved these changes Mar 19, 2019
View reviewed changes
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.

LGTM, thanks!

@not-an-aardvark not-an-aardvark merged commit 12f256f into eslint:master Apr 4, 2019
v6.0.0 automation moved this from Implemented, pending review to Done Apr 4, 2019
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Oct 2, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Oct 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@platinumazure platinumazure platinumazure approved these changes

@mysticatea mysticatea mysticatea approved these changes

Assignees
No one assigned
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 breaking This change is backwards-incompatible enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
No open projects
v6.0.0
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

no-confusing-arrow enable allowParens: true by default
4 participants
@aladdin-add @platinumazure @mysticatea @not-an-aardvark