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

Initial port to Swift 4 #67

Closed
wants to merge 13 commits into from
Closed

Initial port to Swift 4 #67

wants to merge 13 commits into from

Conversation

tcldr
Copy link

@tcldr tcldr commented Jun 19, 2017

What's in this pull request?

Initial migration of library to Swift 4. (Minor changes but lib required by Swiftz which has more extensive changes)

Why merge this pull request?

Migrates library to latest version of Swift. All tests passing with the Swift 4 version of SwiftCheck that I submitted as a PR earlier.

What's worth discussing about this pull request?

This PR focuses on getting the build compiling without warning and passing all tests. I will have missed anything not picked up by these! There may be some wider changes you have in mind to take advantage of Swift 4.

What downsides are there to merging this pull request?

Uses temporary refs to my personal SwiftCheck forks with Swift 4 migration. Will need updating to point to official SwiftCheck repos.

@bkase
Copy link
Member

bkase commented Jul 14, 2017

Alright, I merged the SwiftCheck PR -- so now we can point to that and then we can merge this one!

@tcldr
Copy link
Author

tcldr commented Jul 14, 2017

Nice one. I've updated Operadics to SPM v4 too. Once that's in we can cascade the rest up!

Updated to use SPM v4 directory layout/file format
Updated for Xcode 9, beta 3
Set Travis to run tests on Linux build
@CodaFi
Copy link
Member

CodaFi commented Sep 22, 2017

Xcode 9 redid the file paths so they match the project structure. This is undesirable given that I would like to support just a package build in the future. Could you please try to keep everything in their original positions (modulo moving things into the target directory)?

@@ -0,0 +1,25 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This file should be ignored

Copy link
Author

Choose a reason for hiding this comment

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

Sure. Added to .gitignore along with the Cartfile resolved. My understanding was that these files should be committed however to ensure that those using the project are using the last known working versions of any dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, but Operadics is essentially frozen, and the other two are test-only dependencies. It doesn’t matter which version our own internal repos are built against, it won’t affect our clients.

@@ -0,0 +1,236 @@
//
Copy link
Member

Choose a reason for hiding this comment

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

We should take a dependency on Operadics, not include it directly.

Copy link
Author

Choose a reason for hiding this comment

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

Good spot. Not sure how that got in there.

Cartfile.private Outdated
@@ -1,3 +1,3 @@
github "typelift/Operadics"
github "typelift/SwiftCheck"
github "typelift/Operadics" "swift-develop"
Copy link
Member

Choose a reason for hiding this comment

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

You may remove these. I've just pushed Xcode 9 updates to SwiftCheck and Operadics

Added *.resolved to .gitignore
Updated travis to use Swift 4 release build

Removed unreferenced Operators file

Cleaned up folder hierarchy
@CodaFi
Copy link
Member

CodaFi commented Nov 2, 2017

Resolved by the merge of #68

@CodaFi CodaFi closed this Nov 2, 2017
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

3 participants