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

Replace Route with RouteResponse plus route index #2589

Closed
2 tasks
1ec5 opened this issue Sep 2, 2020 · 1 comment · Fixed by #3182
Closed
2 tasks

Replace Route with RouteResponse plus route index #2589

1ec5 opened this issue Sep 2, 2020 · 1 comment · Fixed by #3182
Assignees
Labels
Core Work related to core navigation and integrations. op-ex Refactoring, Tech Debt or any other operational excellence work.
Milestone

Comments

@1ec5
Copy link
Contributor

1ec5 commented Sep 2, 2020

The original vision behind mapbox/mapbox-directions-swift#391 was that a client such as the navigation SDK would operate mostly in terms of a whole RouteResponse plus a route index into that response. That way, we don’t have to copy attributes of the response into each individual route, as seen in mapbox/mapbox-directions-swift#452. Unfortunately, we never got quite that far in #2350:

  • MapboxNavigationService(route:routeOptions:directions:locationSource:eventsManagerType:simulating:routerType:) should take a RouteResponse, not an individual Route.
  • NavigationViewController(for:routeOptions:navigationOptions:) should take a NavigationService instead of a Route.

This much sounds simple, but there are potentially quite a few knock-on effects throughout the codebase. It’s too late to get these backwards-incompatible changes into v1.0. We can get them into a subsequent minor release as long as we take steps to ensure backwards compatibility. A convenience initializer for MapboxNavigationService can wrap a Route in a RouteResponse, and a convenience initializer for NavigationViewController can wrap a Route in a RouteResponse and MapboxNavigationService.

/cc @mapbox/navigation-ios

@1ec5 1ec5 added the op-ex Refactoring, Tech Debt or any other operational excellence work. label Sep 2, 2020
@1ec5 1ec5 added this to the v1.1.0 milestone Sep 2, 2020
@1ec5 1ec5 modified the milestones: v1.1.0, v1.2.0 Oct 27, 2020
@1ec5 1ec5 modified the milestones: v1.2.0, v1.3.0 Dec 11, 2020
@truburt truburt removed this from the e (android-v1.5.0 / ios-v1.3.0) milestone Feb 26, 2021
@truburt truburt added the Core Work related to core navigation and integrations. label Mar 2, 2021
@1ec5 1ec5 modified the milestones: v2.1 (beta), v2.0.0 (RC) Jul 8, 2021
@Udumft Udumft self-assigned this Jul 13, 2021
@Udumft
Copy link
Contributor

Udumft commented Jul 16, 2021

Corresponding changes happen in these PRs for directions and for NavSDK.

@1ec5 1ec5 linked a pull request Jul 23, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Work related to core navigation and integrations. op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants