-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Refactor FXIOS-10205 [Swiftlint] Enable implicitly_unwrapped_optional rule but keep it disabled for test files using nested Swiftlint configurations #24031
Changes from all commits
7b7e329
a84cde9
0ccfd4f
6ecd083
cdbff29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Swiftlint configuration overloads that will be applied to all files | ||
# in this folder and all its children recursively, unless another | ||
# nested configuration takes over. | ||
# https://github.com/realm/SwiftLint#nested-configurations. | ||
|
||
disabled_rules: | ||
# Many test use implicitly unwrapped optional properties that | ||
# initialized in setUp and reset in tearDown. | ||
- implicitly_unwrapped_optional |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,15 +7,15 @@ import WebEngine | |
|
||
struct EngineProvider { | ||
// We only have one session in the SampleBrowser | ||
private(set) var session: EngineSession? | ||
private(set) var session: EngineSession | ||
let view: EngineView | ||
|
||
init(engine: Engine = WKEngine.factory(), | ||
sessionDependencies: EngineSessionDependencies? = nil) { | ||
init?(engine: Engine = WKEngine.factory(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is intended behavior, LGTM just tagging @lmarceau that worked on this implementation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wow, fast review! As I've mentioned below:
So I couldn't really test this fix, but it was the simplest workaround to get rid of the implicitly unwrapped property. Since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change works for me 👍 |
||
sessionDependencies: EngineSessionDependencies? = nil) { | ||
do { | ||
session = try engine.createSession(dependencies: sessionDependencies) | ||
} catch { | ||
session = nil | ||
return nil | ||
} | ||
|
||
view = engine.createView() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Swiftlint configuration overloads that will be applied to all files | ||
# in this folder and all its children recursively, unless another | ||
# nested configuration takes over. | ||
# https://github.com/realm/SwiftLint#nested-configurations. | ||
|
||
disabled_rules: | ||
# Many test use implicitly unwrapped optional properties that | ||
# initialized in setUp and reset in tearDown. | ||
- implicitly_unwrapped_optional |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,4 +14,5 @@ disabled_rules: | |
- attributes | ||
- trailing_whitespace | ||
- vertical_whitespace | ||
- implicitly_unwrapped_optional | ||
|
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 looks great! Thanks for the work on this issue 🙌 I'll let my colleagues do the final review!