Skip to content
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

Xcode 8.3 and Swift 3.x support #240

Merged
merged 6 commits into from
Mar 30, 2017
Merged

Xcode 8.3 and Swift 3.x support #240

merged 6 commits into from
Mar 30, 2017

Conversation

Liquidsoul
Copy link
Collaborator

@Liquidsoul Liquidsoul commented Mar 29, 2017

This pull request is mostly a merge of the swift-3.0 branch to implement #233 :

  • it makes master use Swift 3.0 as default.
  • it fixes some string warnings in some logs from Xcode 8.3.

Once this is merged we will create a swift-2.3 branch to allow users to use the legacy Swift version if they need it.

One thing I am wondering is if the Travis CI configuration should not be updated in this PR to test both Xcode 8.3 and Xcode 8.2?
Maybe at least change the osx_image to xcode8.3 instead of xcode8 which from travis' documentation targets some Xcode 8 GM
🙄
The build matrix has been updated to use Xcode 8.3.

A language badge has been added to the README file. Language: Swift 3.1

@AliSoftware
Copy link
Owner

I think by now it would be better to have the following branch setup:

  • Make master always use the latest Swift version, even if it's early in adoption (so in our case today, Swift 3.1)
  • Make dedicated branches for older versions, so swift-2.3 and swift-3.0

Because if today we make master use 3.0 and make a swift-2.3 and swift-3.1 branch we'll later again fall back in the similar issue we have today once 3.1 will be widely adopted.

Thoughts?

@AliSoftware
Copy link
Owner

❓One thing I am wondering is if the Travis CI configuration should not be updated in this PR to test both Xcode 8.3 and Xcode 8.2?

Yeah probably 👍

(Just to be clear, we won't need a build matrix to always build master on all Xcode's, just each branch — master, swift-2.3 and swift-3.0 will each have their travis.yml configured with the matching osx_image for the branch)

Copy link
Owner

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a badge on top of the README (next to all the other badges) to indicate the Swift version we support, so users won't be lost

[![Language: Swift 3.1](https://img.shields.io/badge/Swift-3.1-orange.svg)](https://swift.org)

Language: Swift 3.0

(We could retroactively add this badge to each swift-x.y branch as well)

@Liquidsoul
Copy link
Collaborator Author

Is there really a difference between Swift 3.1 and Swift 3.0?
Because it does not seem to be the case or maybe I am missing something 🤔

@AliSoftware
Copy link
Owner

I don't think there is any code change needed at all.
The only requested changes is for Carthage users, again… who will require us to do a branch on which the only difference compared to 3.0 would be the value of SWIFT_VERSION

@AliSoftware
Copy link
Owner

Side note: we need to migrate OHHTTPStubs to Circle-CI some day… travis is still taking forever :'(

@@ -328,6 +333,7 @@
IPHONEOS_DEPLOYMENT_TARGET = 8.3;
MTL_ENABLE_DEBUG_INFO = NO;
SDKROOT = iphoneos;
SWIFT_OPTIMIZATION_LEVEL = "-Owholemodule";
SWIFT_VERSION = 2.3;
Copy link
Owner

@AliSoftware AliSoftware Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line still 2.3? (I'm reading that from the GitHub web page on mobile, so hard to easily get the context of that line in the whole xcodeproj to check if it's normal, but seems strange to me)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the project level settings which is overwritten by the target level settings. I'll change that and remove the target level one.

@AliSoftware AliSoftware merged commit 8660a5a into master Mar 30, 2017
@AliSoftware AliSoftware deleted the Xcode-8.3 branch March 30, 2017 13:20
@AliSoftware
Copy link
Owner

AliSoftware commented Mar 30, 2017

@Liquidsoul Now that this is merged:

  • master is in Swift 3.1
  • I've moved the swift-3.0 branch to be equal to master, so that people referencing that branch (especially people using Carthage) can still use it. We'll remove it as soon as we publish a new release/tag
  • I've created a swift-2.3 branch (that is equal to where master was before the merge of that PR)

Next steps is to:

  • Add a commit to that new swift-2.3 branch to add the shields.io badge in the README
  • Create a new release x.y.z (version to determine)
  • Tag the branch master with x.y.z but also the branch swift-2.3 with x.y.z-swift2.3
  • Remove the swift-3.0 branch
  • Profit!

@Liquidsoul
Copy link
Collaborator Author

@AliSoftware I've just pushed the badge in branch swift2.3.

@AliSoftware
Copy link
Owner

👌

@krunk4ever
Copy link

I noticed that the releases are versioned as:

  • 5.2.2
  • 5.2.2-swift3
  • 5.2.3
  • 5.2.3-swift3

Moving forward, will this be switched to?

  • 5.2.4
  • 5.2.4-swift23

@AliSoftware
Copy link
Owner

Yep exactly that's what I plan indeed. Do you think we should do otherwise? Suggestions welcome.

Also wondering if I should bump the next release to 6.0.0 because the switch from 2.3 to 3.0 is source-breaking (I think, will have to compare our APIs for the two versions to check if the call site has to be changed or not 🤔)…

@krunk4ever
Copy link

I agree you should make 5.2.3 not compatible with the next version since they are not compatible, so 👍 on 6.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants