Skip to content
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

Remove DirectionsResult.routeIdentifier property #452

Closed
1ec5 opened this issue Sep 2, 2020 · 2 comments
Closed

Remove DirectionsResult.routeIdentifier property #452

1ec5 opened this issue Sep 2, 2020 · 2 comments
Labels
blocked Blocked by dependency or unclarity. op-ex Refactoring, Tech Debt or any other operational excellence work.

Comments

@1ec5
Copy link
Contributor

1ec5 commented Sep 2, 2020

#406 moved the UUID from each individual Route to the RouteResponse, aligning with the Directions API. However, it looks like it kept around a vestigial DirectionsResult.routeIdentifier property. We should remove this property. However, if we need to defer this work past v1.0, then we should instead deprecate the property and wait until v2.0 to remove it.

/**
A unique identifier for a directions request.
Each route produced by a single call to `Directions.calculate(_:completionHandler:)` has the same route identifier.
*/
open var routeIdentifier: String?
routeIdentifier = try container.decodeIfPresent(String.self, forKey: .routeIdentifier)

By design, this property is no longer set to anything, but there are still unit tests that assert that it is set to a meaningful value. These tests pass because we haven’t updated the test fixtures.

/cc @mapbox/navigation-ios

@1ec5 1ec5 added backwards incompatible changes that break backwards compatibility of public API op-ex Refactoring, Tech Debt or any other operational excellence work. labels Sep 2, 2020
@1ec5 1ec5 added this to the v1.0.0 milestone Sep 2, 2020
@1ec5 1ec5 mentioned this issue Sep 2, 2020
@1ec5
Copy link
Contributor Author

1ec5 commented Sep 2, 2020

The postprocessing step imbues each route with its surrounding response’s UUID, so the values do match, even if it’s a code smell to use this vestigial property:

route.routeIdentifier = identifier

@1ec5 1ec5 linked a pull request Sep 2, 2020 that will close this issue
@1ec5 1ec5 removed a link to a pull request Sep 2, 2020
@1ec5 1ec5 modified the milestones: v1.0.0, v1.1.0 Oct 2, 2020
@1ec5 1ec5 removed the backwards incompatible changes that break backwards compatibility of public API label Oct 2, 2020
@1ec5 1ec5 added the blocked Blocked by dependency or unclarity. label Oct 27, 2020
@1ec5 1ec5 modified the milestones: v1.1.0, v1.2.0 Oct 27, 2020
@truburt truburt removed this from the e (android-v1.5.0 / ios-v1.3.0) milestone Feb 26, 2021
@Udumft
Copy link
Contributor

Udumft commented Sep 14, 2021

Closed by #562

@Udumft Udumft closed this as completed Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked by dependency or unclarity. op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

No branches or pull requests

3 participants