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

Complexity has no default #4808

Closed
IanVS opened this Issue Dec 26, 2015 · 9 comments

Comments

Projects
None yet
8 participants
@IanVS
Member

IanVS commented Dec 26, 2015

Enabling the complexity rule with only a severity has no effect. We have tried to give sane defaults to all rules, and I think this should be no exception. I don't know what a good number would be, though.

@eslintbot

This comment has been minimized.

Show comment
Hide comment
@eslintbot

eslintbot Dec 26, 2015

@IanVS Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

eslintbot commented Dec 26, 2015

@IanVS Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@gyandeeps

This comment has been minimized.

Show comment
Hide comment
@gyandeeps

gyandeeps Dec 26, 2015

Member

This would be very debatable... My wild guess would be ~4.

Member

gyandeeps commented Dec 26, 2015

This would be very debatable... My wild guess would be ~4.

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Dec 26, 2015

Member

In previous situations, we have tended to use 10, as a nice, round, large numbered default. I'm not sure whether it might make sense to go a bit higher in this case, though. The highest we have within ESLint code is 36, but there are only 6 above 20. Maybe 20 is a good number?

Member

IanVS commented Dec 26, 2015

In previous situations, we have tended to use 10, as a nice, round, large numbered default. I'm not sure whether it might make sense to go a bit higher in this case, though. The highest we have within ESLint code is 36, but there are only 6 above 20. Maybe 20 is a good number?

@SpadeAceman

This comment has been minimized.

Show comment
Hide comment
@SpadeAceman

SpadeAceman Dec 29, 2015

Lower complexity is of course better, but trying to determine a sensible, one-size-fits-all default is tricky.

If we're going for an "only stop me if I'm approaching insanity" sort of default, then it sounds like 20 could indeed work, based on the recommendations and anecdotal evidence I've assembled below. (Myself, I'd lean toward defaulting to 10 or 15.)

SpadeAceman commented Dec 29, 2015

Lower complexity is of course better, but trying to determine a sensible, one-size-fits-all default is tricky.

If we're going for an "only stop me if I'm approaching insanity" sort of default, then it sounds like 20 could indeed work, based on the recommendations and anecdotal evidence I've assembled below. (Myself, I'd lean toward defaulting to 10 or 15.)

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Dec 29, 2015

Member

Awesome research, @SpadeAceman! @nzakas, what is your preference here?

Member

IanVS commented Dec 29, 2015

Awesome research, @SpadeAceman! @nzakas, what is your preference here?

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Dec 29, 2015

Member

Wow, thanks for the research @SpadeAceman. I agree with your conclusion.

Member

michaelficarra commented Dec 29, 2015

Wow, thanks for the research @SpadeAceman. I agree with your conclusion.

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Dec 29, 2015

Member

Sounds like 20 is a good upper bound as the default.

Member

nzakas commented Dec 29, 2015

Sounds like 20 is a good upper bound as the default.

@IanVS IanVS added accepted and removed evaluating labels Dec 31, 2015

@IanVS IanVS closed this in 1288ba4 Dec 31, 2015

nzakas added a commit that referenced this issue Dec 31, 2015

Merge pull request #4836 from eslint/issue4808
Update: Add default limit to `complexity` (fixes #4808)
@Mardak

This comment has been minimized.

Show comment
Hide comment
@Mardak

Mardak Apr 30, 2016

Contributor

As reported in #4809, eslint:recommended configuration has "complexity": ["off", 11], (https://github.com/eslint/eslint/blob/master/conf/eslint.json#L141) so the added "default" of 20 in 1288ba4 doesn't actually get used when using recommended.

Contributor

Mardak commented Apr 30, 2016

As reported in #4809, eslint:recommended configuration has "complexity": ["off", 11], (https://github.com/eslint/eslint/blob/master/conf/eslint.json#L141) so the added "default" of 20 in 1288ba4 doesn't actually get used when using recommended.

@platinumazure

This comment has been minimized.

Show comment
Hide comment
@platinumazure

platinumazure Apr 30, 2016

Member

@Mardak Please open a new issue so we can discuss further. Thanks!

Member

platinumazure commented Apr 30, 2016

@Mardak Please open a new issue so we can discuss further. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.