-
Notifications
You must be signed in to change notification settings - Fork 92
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
Make RouteOptions available on route response #122
Conversation
MapboxDirections/MBRoute.swift
Outdated
@@ -26,12 +26,12 @@ open class Route: NSObject, NSSecureCoding { | |||
- parameter waypoints: An array of waypoints that the route visits in chronological order. | |||
- parameter profileIdentifier: The profile identifier used to request the routes. |
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 parameter has been replaced by routeOptions
.
MapboxDirections/MBRoute.swift
Outdated
@@ -156,18 +156,16 @@ open class Route: NSObject, NSSecureCoding { | |||
open let expectedTravelTime: TimeInterval | |||
|
|||
/** | |||
A string specifying the primary mode of transportation for the route. | |||
|
|||
The value of this property is `MBDirectionsProfileIdentifierAutomobile`, `MBDirectionsProfileIdentifierAutomobileAvoidingTraffic`, `MBDirectionsProfileIdentifierCycling`, or `MBDirectionsProfileIdentifierWalking`, depending on the `profileIdentifier` property of the original `RouteOptions` object. This property reflects the primary mode of transportation used for the route. Individual steps along the route might use different modes of transportation as necessary. |
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.
Developers may not be aware of this nuance, so we might want to leave it in:
The route options object’s
profileIdentifier
property reflects the primary mode of transportation used for the route. Individual steps along the route might use different modes of transportation as necessary.
MapboxDirectionsTests/V5Tests.swift
Outdated
XCTAssertTrue(opts.includesSteps) | ||
XCTAssertTrue(opts.includesAlternativeRoutes) | ||
XCTAssertEqual(opts.routeShapeResolution, .full) | ||
|
||
XCTAssertNotNil(route) |
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.
Move this assertion up above any assertions that depend on route
being non-nil.
MapboxDirectionsTests/V5Tests.swift
Outdated
@@ -52,6 +52,13 @@ class V5Tests: XCTestCase { | |||
XCTAssertEqual(task.state, .completed) | |||
} | |||
|
|||
let opts = route!.routeOptions | |||
|
|||
XCTAssertEqual(opts.profileIdentifier, .automobile) |
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.
Might as well assert that route!.routeOptions
is equal to options
. That way we can be sure they’re literally the same object; no need to compare individual properties on them.
Instead of returning the profile identifier on the route, this replaces it with the entire RouteOptions used to create the directions request.
/cc @frederoni @1ec5