Update for Xcode 9.3/Swift 4.1#244
Update for Xcode 9.3/Swift 4.1#244pcantrell merged 2 commits intobustoutsolutions:swift-4.1from joaomvfsantos:master
Conversation
|
Was going to introduce a PR for the same. Hence, the tests fail on Swift 4.0 and earlier. The solution should be to conditionally use compactMap/flatMap based on the Swift compiler version used. |
|
@edwardmp Hey! Yes that makes sense, I could do those changes. How would you suggest doing this? Using a pre processor macro like this: #if swift(>=4.1)
// .compactMap
#else
// .flatMap
#endifI confess I'm not used to use pre processor macros, as such my only concern is that I would probably duplicate a lot of code (at least the whole block inside the map function). |
|
My usual approach with Swift language updates has been to fold them into the next minor version, and require a higher Swift version for that release while back-porting any critical fixes to a new patch release on the previous minor version. In other words, Siesta 1.4 will require Swift 4.1, and 1.3.x will maintain 4.0 support. So no need for conditional compilation or 4.0 backward compatibility; I'll just keep this PR handy until I'm ready to start putting together Siesta 1.4. |
|
@pcantrell I would normally agree, but since flatMap is used in just 5 places it might make sense to do this the "easy" way. Although I do have to say that a lot of my dependencies cause warnings for the same issue, so I guess I'll be stuck with those warnings for a while! |
|
@joaomvfsantos Hi. In order to avoid code duplication as you said, you can store the closure in a variable. This way you only need one line inside the conditional cases. For Example: let stringNumbers = ["1", "2", "3", "x", "4", "5"]
let closure: (String) -> Int? = { Int($0) }
#if swift(>=4.1)
let numbers = stringNumbers.compactMap(closure)
#else
let numbers = stringNumbers.flatMap(closure)
#endif
print(numbers)Edit: I mean, in this example the closure is just one line too, but it could be a bigger block of code with multiple lines. PS. I was about to make a PR about these annoying warnings lol, good think I checked first. |
|
@Linnk |
|
@edwardmp Well, I was doing it… but then I read the comment about the siesta and swift versions and stuff. And I get it, I'm not fan either of those #if conditions. |
|
@Linnk Thanks for taking the time to give a concrete example. I started to doing an approach like that but after @pcantrell comment I did not proceed. His view also makes sense to me. |
|
If we were going to simultaneously support Swift 4.0 and 4.1, I’d prefer to do it with a conditional extension, which can live in its own separate file and doesn’t clutter up usage sites: #if !swift(>=4.1)
extension Sequence
{
func compactMap<ElementOfResult>(_ transform: (Self.Element) throws -> ElementOfResult?) rethrows -> [ElementOfResult]
{ return try flatMap(transform) }
}
#endifHowever, as I said above, I don’t see any particular reason not to just require 4.1 in the next Siesta release. If somebody does find a compelling reason to keep 4.0 support, we could add the above. For now, if you want to get this PR passing CI, try updating the |
|
Also P.S. I expect to be pulling together Siesta 1.4 in early May. There are some awesome new caching and custom request building features in the works, mostly complete; just have to survive the end of the semester first! If anybody wants a sneak preview, all that work is on the |
|
@pcantrell I changed travis to use the Xcode 9.3 image but it's failing only on iPhone 4s simulator. Any ideas why? |
Basically I just did 2 things:
Note: Travis Build shall fail until it's updated to use Xcode 9.3 image