Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Enable 'prefer-switch' rule and raise default 'min-cases'#2669

Merged
nchen63 merged 2 commits intopalantir:masterfrom
andy-hanson:use_prefer-switch
May 4, 2017
Merged

Enable 'prefer-switch' rule and raise default 'min-cases'#2669
nchen63 merged 2 commits intopalantir:masterfrom
andy-hanson:use_prefer-switch

Conversation

@andy-hanson
Copy link
Contributor

@andy-hanson andy-hanson commented May 3, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Just enables the rule and fixes new lint failures.
EDIT: Also raises the default min-cases.

CHANGELOG.md entry:

[enhancement] prefer-switch: Change default min-cases to 3.

case ts.SyntaxKind.BooleanKeyword:
if (node.initializer.kind === ts.SyntaxKind.TrueKeyword || node.initializer.kind === ts.SyntaxKind.FalseKeyword) {
failure = "boolean";
switch (node.initializer.kind) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could set the threshold to 3
IMO these short switch statements bloat the code unnecessary

default:
return true;
}
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

you could leave this return and remove the default clauses above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the default so it's easy to tell that there's no fallthrough.

@andy-hanson andy-hanson changed the title Enable 'prefer-switch' rule Enable 'prefer-switch' rule and raise default 'min-cases' May 4, 2017
@nchen63 nchen63 merged commit 21db100 into palantir:master May 4, 2017
@andy-hanson andy-hanson deleted the use_prefer-switch branch May 5, 2017 00:19
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