Skip to content

Conversation

@RingerJK
Copy link
Contributor

@RingerJK RingerJK commented Dec 12, 2019

Description

Direction API models map to Navigation SDK's

  • I have added any issue links
  • I have added all related labels (bug, feature, new API(s), SEMVER, etc.)
  • I have added the appropriate milestone and project boards

Goal

Please describe the PR goals. Just the stuff needed to implement the fix / feature and a simple rationale. It could contain many check points if needed

Implementation

Please include all the relevant things implemented and also rationale, clarifications / disclaimers etc. related to the approach used. It could be as self code companion comments

Screenshots or Gifs

Please include all the media files to give some context about what's being implemented or fixed. It's not mandatory to upload screenshots or gifs, but for most of the cases it becomes really handy to get into the scope of the feature / bug being fixed and also it's REALLY useful for UI related PRs screenshot gif

Testing

Please describe the manual tests that you ran to verify your changes

  • I have tested locally (including SNAPSHOT upstream dependencies if needed) through testapp/demo app and run all activities to avoid regressions
  • I have tested via a test drive, or a simulation/mock location app
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have updated the CHANGELOG including this PR

@RingerJK RingerJK added this to the v1.0.0 milestone Dec 12, 2019
@RingerJK RingerJK self-assigned this Dec 12, 2019
.baseUrl(baseUrl)
.alternatives(alternatives)
.annotations(annotations)
.approaches(approaches)!!
Copy link
Contributor

Choose a reason for hiding this comment

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

We should fix (remove @Nullable) annotations in mapbox-java e.g. https://github.com/mapbox/mapbox-java/blob/9bd6158292374fc6dd5f7eba33e7586ca1d76177/services-directions/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java#L568 Builder cannot be null to avoid unnecessary !!. The same thing happens with another methods below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we make it self or create a ticket for Direction API team? what is algorithm for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah feel free to cut a PR in mapbox-java fixing this (should be a pretty straightforward one 😛). We're in charge of maintaining the nav-related services, the Directions API team does know nothing about these wrappers 😅

Copy link
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

@RingerJK
Copy link
Contributor Author

@Guardiola31337 just created 😃

@RingerJK RingerJK merged commit 17551d7 into kyv-onboard-route Dec 16, 2019
@RingerJK RingerJK deleted the kyv-onboard-router-direction-api--nav-models-mappers- branch December 16, 2019 08:58
@Guardiola31337
Copy link
Contributor

just created 😃

Thanks for the quick fix 🙇 We should update #2300 as soon as it gets available in a new MAS release.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants