-
Notifications
You must be signed in to change notification settings - Fork 58
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 stencil to 0.11.0 #83
Conversation
@@ -20,6 +20,7 @@ open class StencilSwiftTemplate: Template { | |||
super.init(templateString: templateStringWithMarkedNewlines, environment: environment, name: name) | |||
} | |||
|
|||
// swiftlint:disable:next discouraged_optional_collection |
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.
Might be a suggestion/PR for Stencil to change that method to dictionary: [String: Any] = [:]
instead ;)
@@ -170,7 +170,7 @@ extension StringFiltersTests { | |||
XCTFail("Code did succeed while it was expected to fail for wrong option") | |||
} catch Filters.Error.invalidOption { | |||
// That's the expected exception we want to happen | |||
} catch let error { | |||
} catch { |
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.
Mmmh. I still personally prefer that to be explicit.
I don't really like the implicit let error
which Swift compiler supports, feels too magic, that's why I always specify catch let error
or catch let e as …
, both makes the statement more understandable imho and allows consistency when you happen to have to pattern-match further like only catch specific error types
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 gives a warning if the catch let error
doesn't add anything. If you want explicit, we should use catch let error as Error
.
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.
Yeah I got that and that's not the style I want / am used to.
I'm used to catch let error
without type when we catch all errors because that way you understand where that error
variable is coming from, while with just catch
it's just magically defined and appears from nowhere explicitly.
But I feel like catch let error as Error
isn't adding much and might even be confusing, feels a bit like if let x = y as Any
vs if let x = y
So overall I personally don't like the rule altogether. I would even wish there were an opposite rule forcing the presence of explicit catch let error
as soon as error
is used in the catch block.
.swiftlint.yml
Outdated
@@ -26,6 +28,7 @@ opt_in_rules: | |||
- sorted_imports | |||
- trailing_closure | |||
- unneeded_parentheses_in_closure_argument | |||
- untyped_error_in_catch |
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.
Not a fan of this one, I prefer explicit over implicit
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.
See comment above.
24fb16c
to
5969d44
Compare
5969d44
to
791708b
Compare
GTM? |
Fixes #74.
Also includes a tiny SwiftLint update, and CocoaPods 1.5.0.