Switch to Alamofire. Fixes #29 [skip ci] #30
Conversation
tmcw
force-pushed
the
alamofire
branch
4 times, most recently
from
34b9799
to
5012b2c
Compare
|
It passes tests! @1ec5 care to review? Main point of "huh" is the working around dispatch_sync https://github.com/mapbox/MapboxDirections.swift/pull/30/files#diff-e76be193f7c380a503afe0e47c1cd829L132 - keeping these calls in would hang the main thread, and think that's because Alamofire is already on the main thread so maybe like it's waiting for itself? Really I have no idea. But I removed the calls willy-nilly and am not sure if it works, only sure that it makes tests pass. |
tmcw
changed the title
| @@ -36,10 +37,7 @@ public class MBDirections: NSObject { | |||
|
|
|||
| private let request: MBDirectionsRequest | |||
| private let configuration: MBDirectionsConfiguration | |||
| private var task: NSURLSessionDataTask? | |||
| public var calculating: Bool { | |||
There was a problem hiding this comment.
We should keep this property for compatibility with MapKit. It's how client code knows that the object is already waiting for a request. Perhaps not the most elegant approach, but we should be deliberate about the places we diverge from MapKit.
There was a problem hiding this comment.
Per voice, given the async'ness of requests this bit of compatibility with MapKit isn't worthwhile: our directions SDK doesn't have any good reason will permit concurrent requests and any kind of serialization would happen on the app level.
|
From a quick glance at the Alamofire codebase, I think you're right that it already handles dispatching to the main queue. RequestKit did not, so we had to dispatch to the main queue ourselves; otherwise, client code would end up executing on the URL session delegate queue. |
|
👍 got rid of the big switch statement I'm going to do the user-agent header addition in this branch, but feel like a v4/v5 strategy refactor is better left for another day, since it's pretty involved and also would diverge from the original API even more. |
|
Testing for the User-Agent header is tricky because Nocilla doesn't support expectations on requests: luisobo/Nocilla#4 |
|
I switched from the |
|
I ended up switching back to OHHTTPStubs/Swift in #47, along with rewriting the library in a way that addresses the weirdness above but also makes it more difficult to integrate Alamofire into the library. |
|
Closing per #29 (comment). |
No description provided.