-
Notifications
You must be signed in to change notification settings - Fork 13
Updating to Swift 3 #30
base: master
Are you sure you want to change the base?
Conversation
.travis.yml
Outdated
@@ -1,5 +1,5 @@ | |||
language: objective-c | |||
osx_image: xcode7.3 | |||
osx_image: xcode8.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.
This is not enough to get it building. The old scripts directory needs to go and this entire file needs to be replaced by a more modern script.
Carthage/Checkouts/SwiftCheck
Outdated
@@ -1 +1 @@ | |||
Subproject commit f2141461f015315cc583222f0bb30ae07da8aad8 | |||
Subproject commit 983b5954f221d36efe3469e8a5155d953034994a |
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.
Please don't manually update the submodules. Edit the cartfile and run carthage update --use-submodules
then commit the result.
Carthage/Checkouts/Swiftz
Outdated
@@ -1 +1 @@ | |||
Subproject commit 78665a0921857ce6b1408bd91a5069931cab0191 | |||
Subproject commit afe8639115abd249c85a477b9dea1f0f8c299815 |
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.
Ditto
|
||
public enum JSONError : JSONErrorType { | ||
case Array([JSONError]) // array of errors | ||
case TypeMismatch(String, String) // expected / actual | ||
case Error(ErrorType, String) // error / message | ||
case Error(Error, String) // error / message |
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.
Canonical case names for Swift 3 structures begin with a lowercase letter. That means Error
should not be a case name.
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.
You mean lowercase enum cases?
I saw that in Swiftx (the swift 3 version) Either still has uppercase members for left and right. I though since these really wanted to indicate types somehow (Array, Object, String ...) the exception would be acceptable. Looked a little odd when I tried. Will do anyways.
@@ -9,9 +9,9 @@ | |||
import Foundation | |||
import Swiftz | |||
|
|||
extension NSData { |
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.
Please remove this file and inline the definition of this extension.
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.
Generally, the codes style in this framework needs to be fixed. We always leave space around colons that indicate types and spaces after but not before colons that indicate parameters.
return Gen.pure(JSONValue.Null) | ||
} | ||
} | ||
let t: Gen<UInt> = UInt.arbitrary |
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.
The indentation of this entire file has changed.
Thank you for working on this. Still, I can't help but think this entire framework can be replaced by a tweet. |
Note will failed until Swiftz publish its spec. See typelift/Swiftz#327
This pull request makes Tyro compatible with Swift 3
Edit: I tried to get travis working but I don't know enough to fix it :(