Split Directions models from the implementation into a separate module #1104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| @@ -152,7 +152,7 @@ public static Builder builder() { | |||
| public abstract List<VoiceInstructions> voiceInstructions(); | |||
There was a problem hiding this comment.
AutoValueImmutableFields: AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at http://goo.gl/qWo9sC
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
Guardiola31337
force-pushed
the
feature/ak-#1103_module_models
branch
from
ecbd861 to
e8e9506
Compare
| @@ -152,9 +152,9 @@ public static Builder builder() { | |||
| public abstract List<VoiceInstructions> voiceInstructions(); | |||
There was a problem hiding this comment.
AutoValueImmutableFields: AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at http://goo.gl/qWo9sC
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
| @@ -152,9 +152,9 @@ public static Builder builder() { | |||
| public abstract List<VoiceInstructions> voiceInstructions(); | |||
There was a problem hiding this comment.
AutoValueImmutableFields: AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at http://goo.gl/qWo9sC
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
| @@ -152,9 +152,9 @@ public static Builder builder() { | |||
| public abstract List<VoiceInstructions> voiceInstructions(); | |||
There was a problem hiding this comment.
AutoValueImmutableFields: AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at http://goo.gl/qWo9sC
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
LukasPaczos
approved these changes
Jan 13, 2020
…ns models dependencies compile only
…ns models dependencies compile only
| @@ -152,9 +152,9 @@ public static Builder builder() { | |||
| public abstract List<VoiceInstructions> voiceInstructions(); | |||
There was a problem hiding this comment.
AutoValueImmutableFields: AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at http://goo.gl/qWo9sC
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
LukasPaczos
force-pushed
the
feature/ak-#1103_module_models
branch
from
be669f7 to
6cbbe8b
Compare
LukasPaczos
force-pushed
the
feature/ak-#1103_module_models
branch
from
6cbbe8b to
cc3371c
Compare
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
| @@ -152,9 +152,9 @@ public static Builder builder() { | |||
| public abstract List<VoiceInstructions> voiceInstructions(); | |||
There was a problem hiding this comment.
AutoValueImmutableFields: AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at http://goo.gl/qWo9sC
|
Muse analysis has completed. New issues were detected with line numbers outside of the pull request change set:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of writing wrappers and mappers internal to Navigation SDK, we could modify
mapbox-javaand split models (likeRouteLegorLegStep) from the implementation (likeMapboxDirectionswhich makes the HTTP requests).Models would be released as one artifact, while the implementation would be another. This way, the whole Nav SDK could reuse the models passed form both onboard and offboard routers while only the
MapboxOffboardRouterwill depend on actual communication details (Retrofit, OkHttp).This approach would simplify the setup by having only one source of directions DTOs while not blocking future modularisation options.
The code in the
mapbox-javarepo does a moderately good job of separating models into separate packages and I think we could make this switch into 2 artifacts seamless, without breaking semver.cc @abhishek1508 @LukasPaczos @korshaknn