[gardening] Clean up do catch blocks #1627
Conversation
otaviolima
commented
Jul 11, 2018
otaviolima
commented
- Remove wildcard pattern matching
- Refactor catches with empty body
- Prefer using try? to ignore errors instead of empty catch body
- Prefer not assigning a variable name
TestFoundation/TestCodable.swift
Outdated
| XCTFail("\(error) for \(calendar)") | ||
| } catch { |
There was a problem hiding this comment.
Should the catch not be 2 lines up?
There was a problem hiding this comment.
Yes, that is correct. I messed up with another change I've made while I was picking the changes to stage before commit. Nice catch :)
Gonna fix and push again.
There was a problem hiding this comment.
Should be fine now.
otaviolima
force-pushed
the
do-catch-refactor
branch
from
0ed34fa to
7940692
Compare
|
@swift-ci please test |
| @@ -93,7 +93,7 @@ internal class _HTTPURLProtocol: _NativeProtocol { | |||
| case (_, nil): | |||
| set(requestBodyLength: .unknown) | |||
| } | |||
| } catch let e { | |||
| } catch { | |||
There was a problem hiding this comment.
e is used inside the catch block for the errorCode(fileSystemError: e) call so this should be left in
|
Fixed the compilation error, tests should pass now. |
otaviolima
force-pushed
the
do-catch-refactor
branch
from
7940692 to
e99ee89
Compare
|
@swift-ci please test |
|
Seems that @swift-ci did not start. |
|
It did, but there was another error: https://ci.swift.org/view/Pull%20Request/job/swift-corelibs-foundation-PR-Linux/1153/consoleFull |
TestFoundation/TestFileManager.swift
Outdated
| XCTFail() | ||
| } catch { | ||
| } |
There was a problem hiding this comment.
Looks to be another issue with the catch being moved down
- Remove wildcard pattern matching - Refactor catches with empty body - Prefer using try? to ignore errors instead of empty catch body - Prefer not assigning a variable name
otaviolima
force-pushed
the
do-catch-refactor
branch
from
e99ee89 to
019cab0
Compare
|
Fixed and ran tests locally, it is passing in my env. Sorry for the hassle. |
|
@swift-ci please test |
|
Looks like it passed ok: https://ci.swift.org/view/Pull%20Request/job/swift-corelibs-foundation-PR-Linux/1154/console |
|
@swift-ci please test and merge |
