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

Update swiftlint rules #3

Merged
merged 1 commit into from
Feb 23, 2017
Merged

Update swiftlint rules #3

merged 1 commit into from
Feb 23, 2017

Conversation

onmyway133
Copy link
Contributor

@onmyway133 onmyway133 commented Feb 23, 2017

Having a lot of warnings can confuse us, so it's great that we can have a common set of rules that we agree on

unused_closure_parameter

This can cause issue like realm/SwiftLint#1175 when the function is generic

closure_parameter_position

This rule is good. However in cases similar to Cartography when the list of closure parameters is long, we tend to move them to a new line

constrain(percentageLabel, titleLabel) {
      (percentageLabel: LayoutProxy, titleLabel: LayoutProxy) in

      // ...
}

empty_parentheses_with_trailing_closure

This rule is good. However in cases we have a constructor that accepts a closure, we tend to call it like let person = Person() { ... } instead of let person = Person { ... } which makes the intention more clear that we 're constructing an object

redundant_string_enum_value

I have no strong opinion on this actually. It is redundant in a way, but more explicit in another way 🤔

enum Deployment: String {
   case staging = "staging"
   case production = "production"
}

min_length

This is good. But since it confuses public typealias T = U with variable name constraints, so I reduce it to 1 for now

Copy link

@JohnSundell JohnSundell left a comment

Choose a reason for hiding this comment

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

🌷

@onmyway133 onmyway133 merged commit b7064ff into master Feb 23, 2017
@onmyway133 onmyway133 deleted the fix/swiftlint branch February 23, 2017 11:52
@vadymmarkov
Copy link
Contributor

As for redundant_string_enum_value, I kinda like to rely on default value.

@onmyway133
Copy link
Contributor Author

@vadymmarkov me too, but for some projects, they may not be the same, so to be safe, I disable it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants