-
Notifications
You must be signed in to change notification settings - Fork 319
Add internal package to libnavigation-core #2839
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
Conversation
a597570 to
107a1d2
Compare
Codecov Report
@@ Coverage Diff @@
## master #2839 +/- ##
============================================
+ Coverage 35.00% 35.05% +0.04%
- Complexity 2107 2108 +1
============================================
Files 541 543 +2
Lines 19422 19390 -32
Branches 1841 1841
============================================
- Hits 6799 6797 -2
+ Misses 11807 11777 -30
Partials 816 816 |
2098246 to
03c679a
Compare
| INCORRECT_AUDIO_GUIDANCE | ||
| ) | ||
| annotation class FeedbackType | ||
| annotation class Type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BicycleType is an enum. It's also for the Onboard module (not actively under development at the moment) so for now I'm going to leave it as is.
DirectionsRouteType is from the legacy.
| UI | ||
| ) | ||
| annotation class FeedbackSource | ||
| annotation class Source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of scope of a package change: https://github.com/mapbox/mapbox-navigation-android/pull/2839/files#r416766318
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| val jobControl = ThreadController.getIOScopeAndRootJob() | ||
| (0 until maxCoroutines).forEach { | ||
| jobControl.scope.launch { | ||
| delay(maxDelay) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
= coroutineRule.runBlockingTest
this will delay the test thread no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh this change was ThreadControllerKtTest to ThreadControllerTest. up to you to update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this doesn't affect the test as the launch is async i.e. the delay is not blocking.
| import com.google.gson.annotations.SerializedName | ||
| import java.util.Date | ||
|
|
||
| internal data class ReplayLocationDto( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋
03c679a to
bb7e637
Compare
| fun postUserFeedback( | ||
| @FeedbackEvent.FeedbackType feedbackType: String, | ||
| @FeedbackEvent.Type feedbackType: String, | ||
| description: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not adding @FeedbackEvent.Description for now cc @JunDai
| override fun postUserFeedback( | ||
| @FeedbackEvent.FeedbackType feedbackType: String, | ||
| @FeedbackEvent.Type feedbackType: String, | ||
| description: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not adding @FeedbackEvent.Description for now cc @JunDai
| private fun postUserFeedbackHelper( | ||
| @FeedbackEvent.FeedbackType feedbackType: String, | ||
| @FeedbackEvent.Type feedbackType: String, | ||
| description: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above re: @FeedbackEvent.Description cc @JunDai
| fun postUserFeedback( | ||
| @FeedbackEvent.FeedbackType feedbackType: String, | ||
| @FeedbackEvent.Type feedbackType: String, | ||
| description: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above re: @FeedbackEvent.Description cc @JunDai



Description
This PR adds internal package to
libnavigation-coreSplitting #2417 in different PRs
Fix #2583
Fix #2417
Fix #2811 (comment)
Refs. #2811 #2820 #2836
As this is going to be a big refactor the plan here is to open small PRs and move stuff incrementally making the review easier. Even trying to do so this first PR included changes in 94 files 😱 (mostly moving files around)
bug,feature,new API(s),SEMVER, etc.)Goal
The goal here is to add
internalpackage to1.0modules and the APIs designed only for local usageImplementation
internalpackageinternalmodifierWill follow up with #2417 (comment) (added
TODOs in the code)Testing
SNAPSHOTupstream dependencies if needed) through testapp/demo app and run all activities to avoid regressionsChecklist
CHANGELOGincluding this PR