Skip to content

Conversation

@Guardiola31337
Copy link
Contributor

We found in mapbox/mapbox-navigation-android#1733 (comment) that naming could be confusing and should be more explicit in order to make clients' life easier.

Copy link
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this update @Guardiola31337

@Guardiola31337 Guardiola31337 merged commit 6e6110a into master Feb 11, 2019
@Guardiola31337 Guardiola31337 deleted the pg-fix-via-waypoints-apis-naming branch February 11, 2019 16:11
@osana
Copy link
Contributor

osana commented Feb 11, 2019

@Guardiola31337
If we are doing this name change, we should do a similar name change in MapboxMapMatching
I guess we can deprecate the use of waypoints and add viawaypoints (for now)

Also please add viawaypoints to RouteOptions

private Point destination;
private Point origin;
private String[] approaches;
private Integer[] waypoints;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was correct as-was. Should be waypoints as one word, no capital P

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're correct. Will update in a follow-up PR. Thanks for flagging 🙇

@Guardiola31337
Copy link
Contributor Author

@osana

If we are doing this name change, we should do a similar name change in MapboxMapMatching
I guess we can deprecate the use of waypoints and add viawaypoints (for now)

Yeah, I agree. We should be consistent across the different services. Will implement ☝️ if we're not cool breaking SemVer.

Also please add viawaypoints to RouteOptions

You mean, adding "waypoints" option to Map Matching route options, right? I've already added it to Directions 👀

public abstract Builder viaWayPoints(@Nullable String indices);

@osana
Copy link
Contributor

osana commented Feb 11, 2019

@Guardiola31337 Please add a call to viaWaypoints() when RouteOptions are being saved/generated:

I believe tests might be updated as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants