-
Notifications
You must be signed in to change notification settings - Fork 26.6k
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
Upgrade to eslint v2 #730
Upgrade to eslint v2 #730
Conversation
@@ -32,7 +32,7 @@ module.exports = { | |||
// disallow else after a return in an if | |||
'no-else-return': 2, | |||
// disallow use of labels for anything other then loops and switches | |||
'no-empty-label': 2, | |||
'no-labels': [2, { 'allowLoop': true, 'allowSwitch': true }], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should definitely set both of these to false
- we don't allow GOTO in any form, including breaking to a label.
Proposal:Add with eslint 2.0.0 upgrade
Set to |
@hshoff at the least, we should include all the new rules with all their options, but set to |
|
// good | ||
class Rey extends Jedi { | ||
constructor(...args) { | ||
super(...args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also "bad" - a constructor that does nothing but passes the args untouched to super
is also a violation of the linter rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
- `id-blacklist` - `no-extra-label` - `no-implicit-globals` - `no-restricted-imports` - `no-unmodified-loop-condition` - `no-unused-labels` - `sort-imports` - `yield-star-spacing`
'no-empty-label': 2, | ||
// disallow Unnecessary Labels | ||
// http://eslint.org/docs/rules/no-extra-label | ||
'no-extra-label': 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we already prevent labels in no-labels
, i think this can be set to 2
without any additional justification in the guide (assuming it already says "no labels ever" somewhere)
3513243
to
80e30a1
Compare
On it - I'm doing some rebasing (also making sure both devDeps and peerDeps are updated in lockstep), I'll force push it up shortly and will publish soon after. |
ee15797
to
244bb13
Compare
} | ||
|
||
return false; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I agree with this example, it doesn't seem to pertain to the text of the rule here.
Also, this example could be simplified to:
[1, 2, 3].filter((x) => {
return x > 2;
});
or even:
[1, 2, 3].filter(x => x > 2);
so I'm not sure it is the best to use here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second one is better in my opinion
97155ea
to
0f32b96
Compare
…origin/harry-eslint-v2' Closes #730.
|
👍 Awesome! |
// good | ||
class Rey extends Jedi { | ||
constructor(...args) { | ||
super(args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the args object be spread into the super constructor? I.e. super(...args)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on what your parent takes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but in this case that would make this class pretty weird. I mean it's a made up example, so it doesn't really matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 8a6f009
To the future we go. Couple more steps to go before publish: