- 
                Notifications
    You must be signed in to change notification settings 
- Fork 42
Upgrade to Swift 5 and apply Xcode fixes #35
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
Conversation
| Codecov Report
 @@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   96.29%   96.29%           
=======================================
  Files          36       36           
  Lines         864      864           
=======================================
  Hits          832      832           
  Misses         32       32Continue to review full report at Codecov. 
 | 
        
          
                Package.swift
              
                Outdated
          
        
      | .package(url: "https://github.com/Quick/Nimble", .upToNextMajor(from: "7.0.0")), | ||
| .package(url: "https://github.com/Quick/Quick", .upToNextMinor(from: "1.2.0")), | ||
| .package(url: "https://github.com/Quick/Nimble", from: "8.0.0"), | ||
| .package(url: "https://github.com/Quick/Quick", from: "2.0.0"), | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should be from: "2.1.0", since Carthage users are free to start depending on hypothetical 2.1 features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| .testTarget(name: "MobiusTestTests", dependencies: ["MobiusTest", "Quick"], path: "MobiusTest/Test"), | ||
| ], | ||
| swiftLanguageVersions: [.v4_2] | ||
| swiftLanguageVersions: [.v5] | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[.v4_2, .v5] for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that? This Package.swift will be used when targeting Swift 5, and Package@swift-4.2.swift when targeting Swift 4.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad brain moment there
Mobius.swift is using a pretty old version of Quick and Nimble which is causing SPM to not resolve the dependency graph in some cases (for example when used to together with XCLogParser which uses Commandant which itself has a dependency on Quick 2.0.0 and Nimble 8.0.0). For this reason, it's good to move to the newer versions.
I took the opportunity to also update to Swift 5 and apply all Xcode 10.2.1 suggested fixes in4d5d5bc.
Update: Turns out these changes are not needed at all, it probably was a temporary Xcode glitch.
Note: the new version of Quick that we have updated to has some warnings that are still unresolved (Quick/Nimble#661).Update: These warnings don't show up anymore on my machine for some reason, even better.