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

Disabling ignoring unused vars for _ prefix #1719

Closed
gaearon opened this issue Mar 5, 2017 · 14 comments
Closed

Disabling ignoring unused vars for _ prefix #1719

gaearon opened this issue Mar 5, 2017 · 14 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Mar 5, 2017

We currently ignore unused vars with _ prefix, mainly as a workaround for deleting extra properties in ...rest destructuring. However #1705 is a proper fix for this, so I think we should remove the _ rule in the next major bump. This would fix #1387.

@gaearon gaearon added this to the 0.10.0 milestone Mar 5, 2017
@gaearon
Copy link
Contributor Author

gaearon commented Mar 6, 2017

Also, why do we have vars: 'local' rather than the default 'global' setting?

@sidoshi
Copy link
Contributor

sidoshi commented Mar 7, 2017

I can submit a PR.
Though I am not sure about the vars setting. Can't find a use case of having an unused global variable.

@gaearon
Copy link
Contributor Author

gaearon commented Mar 7, 2017

If you could go through the git blame history for who added it and why, that could help. Or maybe there was no good reason at all.

@sidoshi
Copy link
Contributor

sidoshi commented Mar 7, 2017

Seems like there was no good reason.
It was there since custom eslint rules were added in the begining. See #27

@gaearon
Copy link
Contributor Author

gaearon commented Mar 7, 2017

We probably took it here then. Would be nice to learn why Airbnb config specifies this.

@gaearon
Copy link
Contributor Author

gaearon commented Mar 7, 2017

Maybe @ljharb could say!

@jamesblight
Copy link
Contributor

jamesblight commented Mar 8, 2017

The default setting in eslint was local at the time Airbnb set their config. Eslint later changed the default to all (global) and I'm guessing Airbnb never updated.

Edit: For context this is why it changed from local to all

@sidoshi
Copy link
Contributor

sidoshi commented Mar 8, 2017

@gaearon should the vars setting be set to default?

@gaearon
Copy link
Contributor Author

gaearon commented Mar 8, 2017

Yea.

@sidoshi
Copy link
Contributor

sidoshi commented Mar 8, 2017

see #1763

@ljharb
Copy link

ljharb commented Mar 15, 2017

This setting certainly predates my involvement with the Airbnb config. I can't see any reason not to change it to "all" - can someone file an issue, or better, send a PR explaining the rationale?

@gaearon
Copy link
Contributor Author

gaearon commented May 11, 2017

@doshisid Do you mind following up with @ljharb and raising an issue in Airbnb ESLint config repo too?

@gaearon
Copy link
Contributor Author

gaearon commented May 11, 2017

Fixed by #1763.

@gaearon gaearon closed this as completed May 11, 2017
@sidoshi
Copy link
Contributor

sidoshi commented May 11, 2017

Sure, I'll do it.

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants