Various minor changes to XCTest-files. #80
Conversation
`XCTMain` is Swift-specific, and should therefore be surrounded by backticks.
Adding backticks to the literal value `false` seemed coherent with other literal values (like array literals `[1, 2, 3]`). The change from `Bool` to `Boolean` was made, as the "general [...] expression" is defined as follows: `@autoclosure expression: () throws -> Boolean`. Therefore `Boolean` seems more accurate.
Changed `selectedTestFilter` to use the `guard` statement, in order to reduce nesting of `if let`-statements. The use of `guard` seems appropriate, as the ultimate goal is to reach `selectedTest.matches` (the previous optional bindings being preconditions for this).
Changed the `map` and `filter` methods, to use trailing closure syntax. This improves the readability, as it removes unnecessary braces.
- Removed braces, which were placed around the item being switched on. This seems to be more confrom to the general Swift-syntax. - If the statements in each case fit on one line, I moved them next to the `case .Something:`- label. If not they are all on a second line. - Removed an unnecessary line break.
- Changed the uses of `map` to use trailing closure syntax, in order to improve readability. - Removed a redundant annotation - Added backticks around Swift-specific types in comments - Split a very long one-line comment into multiple lines. - Changed very simple if-statements to ternary-operators. This also allows for `let` instead of `var`.
|
Thanks, @marcusrossel! I'm supportive of many of these changes. 💯 |
| guard let selectedTestName = selectedTestName else { return includeAllFilter() } | ||
| guard let selectedTest = SelectedTest(selectedTestName: selectedTestName) else { return excludeAllFilter() } | ||
|
|
||
| return selectedTest.matches |
There was a problem hiding this comment.
@briancroom Any opinions here? I find both equally readable.
There was a problem hiding this comment.
I like this change! The flatter structure is nice.
|
Cool! Thanks for taking the time to look through the code so thoroughly @marcusrossel. I've followed up on a couple points in the comments. |
| /// This function emits a test failure if the general Bool expression passed | ||
| /// to it evaluates to false. | ||
| /// This function emits a test failure if the general `Boolean` expression passed | ||
| /// to it evaluates to `false`. |
There was a problem hiding this comment.
I would suggest either keeping it "Bool" and adding the backticks since that is a type name or changing to Boolean and not adding the backpacks since then it's just prose...
There was a problem hiding this comment.
I understand your point. What I don't understand though is: if Boolean is not a defined type, how can it be used as one?:
@autoclosure expression: () throws -> Boolean
This code was taken from XCTAssert.swift
There was a problem hiding this comment.
You're absolutely right! I forgot about the Boolean protocol as distinct from the Bool concrete type.
Never mind!
There was a problem hiding this comment.
Possibly contributing to the confusion is the fact that this was previously named BooleanType before the recent Swift 3 stdlib naming changes.
There was a problem hiding this comment.
Ah, I see. I haven't been aware of that name change.
So, I'll keep the changes, as they were (if that's the consensus).
|
I think this is good to merge. @mike-ferris-apple, could you ask @swift-ci to please test? |
|
@swift-ci please test |
|
Thanks, @marcusrossel! And thanks for the reviews everyone! |
|
Thanks for helping me out guys/gals. |
This pull request contains multiple minor changes:
This pull request mainly aims at making the files more consistent, and readable.