Conversation
|
Added the DO NOT MERGE label as it seems like it's not ready on the backend yet (I"m clarifying). |
|
|
dzinad
force-pushed
the
NAVAND-774-dd
branch
2 times, most recently
from
aec25b4 to
2068495
Compare
Codecov Report
@@ Coverage Diff @@
## main #1503 +/- ##
============================================
+ Coverage 76.89% 76.90% +0.01%
Complexity 937 937
============================================
Files 129 129
Lines 4016 4019 +3
Branches 582 582
============================================
+ Hits 3088 3091 +3
Misses 678 678
Partials 250 250
|
|
Removing the DO NOT MERGE label as it's already supported and working in my example. |
| * @return list of {@link DirectionsWaypoint} objects ordered from start of route till the end | ||
| */ | ||
| @Nullable | ||
| public abstract List<DirectionsWaypoint> waypoints(); |
There was a problem hiding this comment.
the following invokes have a very similar signatures, which might lead to mistakes
val waypoints = directionResponse.waypoints()val waypoints = directionsRoute.waypoints()let's maybe rename it to
| public abstract List<DirectionsWaypoint> waypoints(); | |
| public abstract List<DirectionsWaypoint> routeWaypoints(); |
There was a problem hiding this comment.
But the parameter name is "waypoints". And you invoke it either on the route object or on the response.
What's more, we've discussed that native waypoints will be the source of truth in the SDK.
There was a problem hiding this comment.
But the parameter name is "waypoints"
we can use annotation @SerializedName 😃
And you invoke it either on the route object or on the response.
What's more, we've discussed that native waypoints will be the source of truth in the SDK.
the SDK is not the only customer of the mapbox-java, it might be used without the SDK.
I'm just worrying that signatures are really similar and even if you know where your data is, you might make a mistake
There was a problem hiding this comment.
we can use annotation @SerializedName 😃
I know but I'm just not sure we should use it in this situation and duplicate the knowledge that it's a route waypoint.
I'm just worrying that signatures are really similar and even if you know where your data is, you might make a mistake
But we also have distance and duration for DirectionsRoute, RouteLeg and LegStep.
There was a problem hiding this comment.
ok, it's not a blocker, just a concern. I would hear other opinions, but if there are no any I'm good to keep it as is 👍
| * | ||
| * @return list of {@link DirectionsWaypoint} objects ordered from start of route till the end | ||
| */ | ||
| @Nullable |
There was a problem hiding this comment.
| @Nullable | |
| @NonNull |
I'm dropping this suggestion but let's double-check with the Nav API team.
There was a problem hiding this comment.
Discussed that it's actually Nullable.
* Support waypoints per route (#1503) * changelog v6.10.0-beta.1 Co-authored-by: Dzina Dybouskaya <32109537+dzinad@users.noreply.github.com>
3 participants
No description provided.