AliSoftware / OHHTTPStubs

Stub your network requests easily! Test your apps with fake network data and custom response time, response code and headers!
MIT License
5.03k stars 601 forks source link

Update Swift 5 #306

Closed CRoig closed 5 years ago

CRoig commented 5 years ago

Checklist

Description

I have update Swift default version to 5.0

Motivation and Context

Updating project to Swift 5.0 removes a Carthage warning when updating packages using XCode 10.2.

***  Skipped installing OHHTTPStubs.framework binary due to the error:
    "Incompatible Swift version - framework was built with 4.2.1 (swiftlang-1000.11.42 clang-1000.11.45.1) and the local version is 5.0 (swiftlang-1001.0.69.5 clang-1001.0.46.3).

I built OHHTPPStubs using Carthage using Swift 5.0. Warning is not longer appearing. Also I verified the changes did not break any existing unit test.

CRoig commented 5 years ago

Updated README with Swift5 compatibility and added XCode 10.2 image to the rakefile. No change required on podspec since XCode 10.2 still supports 8.0 iOS.

AliSoftware commented 5 years ago

Thanks!

I just realized that indeed for OHHTTPStubs we haven't updated the podspec with the latest recommendations by CocoaPods as I thought we did. That's is, now CocoaPods supports a swift_version attribute that you can add to your podspec to indicate which version(s) of Swift is supported by the pod. In our case that would mean add this attribute to the 'Swift' subspec then.

That swift_version has been introduced in CocoaPods 1.4, and in CocoaPods 1.7 they introduced support for a range of Swift versions as well using swift_versions. So that migration of OHHTTPStubs to Swift 5 might be the occasion to take advantage of that in our podspec 😉

AliSoftware commented 5 years ago

Mmmh having SWIFT_VERSION set to 5.0 in the project prevents it to be built using older Xcodes, which is why the CI fails. We need to address that before considering merging

jeffctown commented 5 years ago

@AliSoftware Would you be cool using swift_version for this pull request, considering that Cocoapods 1.7.0 is still in beta? I think it makes sense to update to the ranged version of this parameter (swift_versions) after the new Cocoapods is fully released.

I like your suggestion to add swift_version to the Swift subspec, but don't think you can add that parameter to a subspec, or at least thats what Cocoapods complained about when I tried to add it there and use it in the Swift example. I saw this error (using Cocoapods v1.6.1):

Screen Shot 2019-04-09 at 7 33 59 PM

As for getting CI to pass: We can just add the swift version as a rake parameter to override the project setting to set the swift version to 4.0 and 4.2. I have a branch that is passing using that method.

@CRoig would you be okay to grant me access to your fork? I'd like to update some of the project settings the swift migration added before we merge this in. I can also add the changes to get the CI passing.

AliSoftware commented 5 years ago

Yep looks like a good compromise indeed 👍

CRoig commented 5 years ago

@jeffctown No problem at all. I did not have the time yet to fix this. But feel free to fork and amend the PR. Thanks.

jeffctown commented 5 years ago

@AliSoftware looks good to me. Let me know what you think.

AliSoftware commented 5 years ago

🎉

jeffctown commented 5 years ago

@CRoig - I just finished the 8.0.0 release for Swift 5. Thanks for the help!

@eMdOS - FYI