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

Update to Swift 4.2 #541

Closed
wants to merge 5 commits into from
Closed

Conversation

rhysforyou
Copy link

@rhysforyou rhysforyou commented Sep 14, 2018

This PR updates to SnapKit to build with Swift 4.2, which involves a few key changes:

  • Update the Swift version in the project's build settings
  • Update to recommended project settings as suggested by Xcode 10
  • Removes the LayoutAttribute and LayoutRelation attributes used for cross platform compatibility, as now all platforms use NSLayoutConstraint.Attribute and `NSLayoutConstraint.Relation
  • Specifies an explicit Swift version in the project's Podspec
  • Updates the Swift Package Manager manifest to the latest format
  • Update the Travis CI build script to use Xcode 10

I have Travis running against my fork of the repo, you can see build results here: https://travis-ci.org/rhysforyou/SnapKit

@spadafiva
Copy link

@rhysforyou I would love to see this added! Thanks for doing this.

@@ -25,18 +25,9 @@ import Foundation

#if os(iOS) || os(tvOS)
import UIKit
#if swift(>=4.2)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused with why you removed this code? Wouldn't this break support for anything below Swift 4.2 ?

@freak4pc
Copy link
Contributor

Looking into the develop branch (and also just built for myself on my project) - this was already resolved in #543. Seems like this PR is not needed, let me know if you think otherwise?

@avery-pierce
Copy link

Sorry, I'm confused. It looks like issue #543 wasn't resolved?

In any case, I'm trying to use Swift 4.2 in my project, and as of now, adding SnapKit as a CocoaPods dependency causes my app to not compile. Using https://github.com/rhysforyou/SnapKit/tree/swift-4.2 as a remote dependency resolves it.

@freak4pc
Copy link
Contributor

freak4pc commented Sep 17, 2018

@aapierce0 Sorry, I meant #516, and I meant the develop branch.
Can you try adding:

pod 'SnapKit', git: 'https://github.com/SnapKit/SnapKit', branch: 'develop'

@avery-pierce
Copy link

@freak4pc Aha, that seems to work. Thanks!

@Kalvin126
Copy link

Could we set the spec.swift_version = '4.2' in the SnapKit.podspec as well?

@freak4pc
Copy link
Contributor

I don't see why you'd want to do that. It'd break backward compatibility for Swift 4 projects.
Better to leave as is, in which case it would be based on the Project's version.

@Kalvin126
Copy link

Kalvin126 commented Sep 17, 2018

I mean I'd wish the current podspec had spec.swift_version = '4.0', because my project is using Swift 4.2, but SnapKit and other pods are not yet 4.2 ready and CocoaPods is resolving to use my project's Swift version which is 4.2.

I do understand there are problems with both approaches. Right now, I'd have to have post install script to set SnapKit's swift version to 4.0

@freak4pc
Copy link
Contributor

Did you read the comments to this thread? More specifically the one directing to the develop branch ?

@Kalvin126
Copy link

Kalvin126 commented Sep 17, 2018

I did, but I'd rather point to tags instead of branches

@Panajev
Copy link

Panajev commented Sep 17, 2018 via email

@Kalvin126
Copy link

oh, spec.swift_version = '4.2' was actually added in this PR

@fassko
Copy link

fassko commented Sep 21, 2018

@freak4pc

Maybe for older Swift versions we can set up branch or tag and include spec.swift_version = '4.2' in this new release?

@freak4pc
Copy link
Contributor

@fassko I'm not sure what you mean exactly. I never saw a good reason to use spec.swift_version in the podspec level as I feel its a bit aggressive. If the project can be built on both Swift 4 and Swift 4.2, the absence of that is better so the decision is made by the project itself.

@fassko
Copy link

fassko commented Sep 21, 2018

@freak4pc I think you're right, makes total sense. Thanks!

@freak4pc
Copy link
Contributor

4.0.1 was released : https://github.com/SnapKit/SnapKit/releases/tag/4.0.1
I would close this PR If I were you, as it seems Xcode 10 backward-compatible support has been addressed.

@mattbarker016
Copy link

mattbarker016 commented Sep 25, 2018

There's still a warning that SnapKit 4.0.1 needs to be converted to Swift 4.2 in Xcode.

@mattbarker016
Copy link

Why was this closed? There's still a warning that SnapKit 4.0.1 needs to be converted to Swift 4.2.

@likeSo
Copy link

likeSo commented Sep 26, 2018

image

@freak4pc
Copy link
Contributor

This change is wrong @likeSo - These two definitions aren't supported below Swift 4.2.

@likeSo
Copy link

likeSo commented Sep 26, 2018

You mean NSLayoutConstraint.Relation can not be used on Swift 4.2?😀 But i can see that in Xcode:
image
SnapKit version in my Podfile is 4.0.1, and my project still needs to migrate:
image
After conversion, that two changes made...

@freak4pc
Copy link
Contributor

Made a PR to fix the minor warning bothering y'all :) #547

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.

None yet

9 participants