Skip to content

[eslint config][major][guide] Always require parens around arrow function arguments #1863

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
Merged
ljharb merged 1 commit into from
Aug 9, 2019
Merged

[eslint config][major][guide] Always require parens around arrow function arguments #1863

ljharb merged 1 commit into from
Aug 9, 2019

Conversation

@sharmilajesupaul
Copy link
Contributor

@sharmilajesupaul sharmilajesupaul commented Jul 10, 2018

guide + eslint config change to always require parens around arrow function arguments.

@sharmilajesupaul sharmilajesupaul requested a review from ljharb July 10, 2018 02:14
@sharmilajesupaul sharmilajesupaul force-pushed the shar--always-use-parens-for-arrow-functions branch 2 times, most recently from 0ca14dc to 21ef5e4 Compare July 10, 2018 02:16
@tunnckoCore
Copy link

tunnckoCore commented Jul 10, 2018

Yeaaaah!! 🎉 :)

@ljharb
Copy link
Collaborator

ljharb commented Jul 10, 2018

Thanks - since this is a semver-major, I'm going to hold off on merging it until after #1834 is released.

@ljharb ljharb self-assigned this Jul 10, 2018
ljharb
ljharb requested changes Jul 10, 2018
View reviewed changes
Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

LGTM, but blocking merges until it's ready

lencioni
lencioni approved these changes Jul 10, 2018
View reviewed changes
README.md
- [8.4](#arrows--one-arg-parens) Always include parentheses around arguments for clarity and consistency. eslint: [`arrow-parens`](https://eslint.org/docs/rules/arrow-parens.html)
> Why? Less visual clutter.
> Why? Minimizes diff churn when adding or removing arguments.
Copy link
Contributor

@lencioni lencioni Jul 10, 2018

Choose a reason for hiding this comment

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

This is a much better reason!

@sharmilajesupaul sharmilajesupaul force-pushed the shar--always-use-parens-for-arrow-functions branch from 21ef5e4 to 020bdde Compare July 10, 2018 17:07
@ljharb ljharb force-pushed the shar--always-use-parens-for-arrow-functions branch from 020bdde to cf6219d Compare August 13, 2018 18:22
@alexzherdev
Copy link

alexzherdev commented Oct 27, 2018

Since #1834 is done now, can this be rebased and merged?

@ljharb ljharb mentioned this pull request Nov 29, 2018
Merged
@ljharb ljharb force-pushed the shar--always-use-parens-for-arrow-functions branch from bb12bcd to 95f33e7 Compare November 29, 2018 01:18
@aboyton aboyton mentioned this pull request Dec 6, 2018
Closed
j-f1
j-f1 reviewed Dec 6, 2018
View reviewed changes
@ljharb ljharb force-pushed the shar--always-use-parens-for-arrow-functions branch from 95f33e7 to cf57435 Compare December 6, 2018 23:44
@ljharb ljharb force-pushed the shar--always-use-parens-for-arrow-functions branch from e1cef35 to 820745d Compare August 9, 2019 23:09
ljharb
ljharb approved these changes Aug 9, 2019
View reviewed changes
Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

it is time

@ljharb ljharb merged commit 820745d into master Aug 9, 2019
@ljharb ljharb deleted the shar--always-use-parens-for-arrow-functions branch August 9, 2019 23:44
@tunnckoCore
Copy link

tunnckoCore commented Aug 10, 2019 •
edited
Loading

Sweeet. One more rule which I like and forced in my config, got merged. :)
So I guess, I'm not that bad at decisions and logical & good style ;p

@gautier-lefebvre gautier-lefebvre mentioned this pull request Aug 11, 2019
Merged
@ljharb ljharb mentioned this pull request Aug 14, 2019
Closed
ljharb added a commit that referenced this pull request Aug 14, 2019
@ljharb
[guide] clean up confusing example from bad rebase
295d1e6 
 ... from
820745d#diff-04c6e90faac2675aa89e2176d2eec7d8R963-R965
/ #1863.

Fixes #2071.
vunb pushed a commit to hsdt/javascript that referenced this pull request Nov 27, 2019
@ljharb @vunb
[guide] clean up confusing example from bad rebase
8aaa60f 
 ... from
airbnb@820745d#diff-04c6e90faac2675aa89e2176d2eec7d8R963-R965
/ airbnb#1863.

Fixes airbnb#2071.
@elyobo
Copy link

elyobo commented Dec 6, 2019

Overriding this

Why? Minimizes diff churn when adding or removing arguments

I don't find this to be the case; there's still a diff on the same line, so doesn't really make any meaningful change to the diffs, but 👇 is still meaningful.

Why? Less visual clutter.

Glad that it makes some people happy though, and I don't mind overriding things if necessary :)

eugene2candy pushed a commit to eugene2candy/javascript that referenced this pull request Apr 10, 2021
@ljharb @eugene2candy
[guide] clean up confusing example from bad rebase
4f37be8 
 ... from
airbnb@820745d#diff-04c6e90faac2675aa89e2176d2eec7d8R963-R965
/ airbnb#1863.

Fixes airbnb#2071.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb approved these changes

+2 more reviewers

@j-f1 j-f1 j-f1 left review comments

@lencioni lencioni lencioni approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ljharb ljharb

Labels

semver-breaking: guide change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@sharmilajesupaul @tunnckoCore @ljharb @alexzherdev @elyobo @lencioni @j-f1