-
Notifications
You must be signed in to change notification settings - Fork 58
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
Swift 4 migration #16
base: master
Are you sure you want to change the base?
Conversation
The files in |
Thanks! Working on it. |
I believe I've got the templates fixed. I re-generated and ran tests, and all seems clean. |
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.
If you regenerate the models, the date in the comment atop the file should also change. Did you run the script?
tzhour = Int(hourStr.substring(to: hourStr.index(hourStr.startIndex, offsetBy: 2)))! | ||
tzmin = Int(hourStr.substring(from: hourStr.index(hourStr.startIndex, offsetBy: 2)))! | ||
else if 4 == hourStr.count { | ||
tzhour = Int(hourStr[..<hourStr.index(hourStr.startIndex, offsetBy: 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.
Please change to tab indentation
@@ -276,7 +276,7 @@ class DateTimeTests: XCTestCase { | |||
XCTAssertEqual(UInt8(33), d!.time!.minute) | |||
XCTAssertEqual(29, d!.time!.second!) | |||
XCTAssertFalse(nil == d!.timeZone) | |||
XCTAssertEqual(TimeZone.current, d!.timeZone!, "Should default to local time zone but have \(d!.timeZone)") | |||
XCTAssertEqual(TimeZone.current, d!.timeZone!, "Should default to local time zone but have \(String(describing: d!.timeZone))") |
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.
Spaces ➔ tabs (and below, too)
@@ -276,7 +276,7 @@ class DateTimeTests: XCTestCase { | |||
XCTAssertEqual(UInt8(33), d!.time!.minute) | |||
XCTAssertEqual(29, d!.time!.second!) | |||
XCTAssertFalse(nil == d!.timeZone) | |||
XCTAssertEqual(TimeZone.current, d!.timeZone!, "Should default to local time zone but have \(d!.timeZone!)") | |||
XCTAssertEqual(TimeZone.current, d!.timeZone!, "Should default to local time zone but have \(String(describing: d!.timeZone!))") |
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.
Spaces ➔ tabs (and below)
@@ -191,7 +191,7 @@ public struct FHIRInstantiationContext { | |||
public mutating func finalize(for json: FHIRJSON) { | |||
for key in json.keys { // cannot use filter() because that uses a closure, which complains of caturing a mutating foobar | |||
if !containsKey(key) { | |||
addError(FHIRValidationError(unknown: key, ofType: type(of: json[key]!))) | |||
addError(FHIRValidationError(unknown: key, ofType: Swift.type(of: json[key]!))) |
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 think the Swift namespace is not needed here, is it?
This is the result of running Swift-FHIR through Xcode 9's Swift 4 migration. Additionally, I've silenced some deprecation warnings from Xcode 9.1 related to changes in String.