Agios 525.swift3.release #91
Conversation
|
@jcesarmobile Could you take a look at this please? |
|
@corinnekrych Would you also mind having a look please? |
|
@aidenkeating I think we should revisit the Swift3 API (open method) for ex: to follow Swift3 API design: @aidenkeating we missed that in this PR: @jcesarmobile do you agree? |
| @@ -69,7 +69,7 @@ open class DeviceRegistration: NSObject, URLSessionTaskDelegate { | |||
| self.session = Foundation.URLSession(configuration: sessionConfig, delegate: self, delegateQueue: OperationQueue.main) | |||
| } | |||
|
|
|||
| open func overridePushProperties(_ pushProperties: [String: String]) { | |||
| open func overridePushProperties(pushProperties: [String: String]) { | |||
There was a problem hiding this comment.
remane to override(pushProperties:
the PushProperties becomes redundant in the function name since we have a label as first parram
| @@ -204,7 +204,7 @@ open class DeviceRegistration: NSObject, URLSessionTaskDelegate { | |||
| We need to 'override' that 'default' behaviour to return the original attempted NSURLRequest | |||
| with the URL parameter updated to point to the new 'Location' header. | |||
| */ | |||
| open func urlSession(_ session: URLSession, task: URLSessionTask, willPerformHTTPRedirection redirectResponse: HTTPURLResponse, newRequest redirectReq: URLRequest, completionHandler: (@escaping (URLRequest?) -> Void)) { | |||
| open func urlSession(session: URLSession, task: URLSessionTask, willPerformHTTPRedirection redirectResponse: HTTPURLResponse, newRequest redirectReq: URLRequest, completionHandler: (@escaping (URLRequest?) -> Void)) { | |||
There was a problem hiding this comment.
this one keep _ because it is inherited.
|
in https://github.com/aerogear/aerogear-ios-push/blob/master/push-sdk-swift/PushAnalytics.swift#L37 and https://github.com/aerogear/aerogear-ios-push/blob/master/push-sdk-swift/PushAnalytics.swift#L55 |
|
@corinnekrych Are the PushAnalytics changes you recommend not made here? https://github.com/aerogear/aerogear-ios-push/pull/91/files#diff-07bdc4a2fe51d48489af20c79e9379d3L37 |
|
@aidenkeating yes! all good ignore that last commnet |
|
+1 LGTM |
No description provided.