Skip to content
This repository has been archived by the owner on Nov 19, 2017. It is now read-only.

[no-unused-vars]: Disallow declaration of variables that are not used in the code. #1

Merged

Conversation

cjcenizal
Copy link

@cjcenizal cjcenizal commented Jul 5, 2016

Addresses elastic/kibana#7609

Relevant rule: http://eslint.org/docs/rules/no-unused-vars

There are 1070 instances of unused vars in Kibana, according to this rule. Let's kill 'em!

@@ -55,7 +55,8 @@ module.exports = {
'no-undef': 2,
'no-underscore-dangle': 0,
'no-unused-expressions': 0,
'no-unused-vars': 0,
// Disallow declaration of variables that are not used in the code.
'no-unused-vars': [2, { vars: 'local', args: 'after-used' }],
Copy link

Choose a reason for hiding this comment

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

According to the docs, after-used is the default setting, so I don't think we need to set it again.

@epixa
Copy link

epixa commented Jul 5, 2016

LGTM after that one thing

@cjcenizal cjcenizal force-pushed the 7609/improvement/eslint-unused-vars branch from 2b3cc7e to 461321d Compare July 5, 2016 15:07
@Bargs
Copy link

Bargs commented Jul 5, 2016

What's the advantage of after-used over all for args? In what situation would we want unused arguments? I would think that that could only lead to a confusing API for client code.

@epixa
Copy link

epixa commented Jul 5, 2016

@Bargs We don't have control over all crappy APIs though. Some dependency could expose something stupid like introduce an iterator function that puts key first, and then we'd be forced to supply a first argument even though we don't want to use it.

@Bargs
Copy link

Bargs commented Jul 5, 2016

Good point @epixa, makes perfect sense.

LGTM

@cjcenizal cjcenizal merged commit 29da50e into elastic:master Jul 5, 2016
@cjcenizal cjcenizal deleted the 7609/improvement/eslint-unused-vars branch July 5, 2016 17:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants