Add internal package to 1.0 modules #2811

Guardiola31337 · 2020-04-20T22:43:46Z

Description

This PR adds internal package to libdirections-hybrid, libdirections-offboard, libdirections-onboard and libnavigation-base. It also bumps dokka dependency version to 0.10.1

Splitting #2417 in different PRs

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 67 files (mostly moving files around)

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

The goal here is to add internal package to 1.0 modules and the APIs designed only for local usage

Implementation

Move classes around to internal package
Add internal modifier
Fix package across codebase
Move RouteProgressObserver into libnavigation-base
Bump dokka dependency version to 0.10.1 in dependencies.gradle

Testing

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
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

codecov-io · 2020-04-20T22:48:08Z

Codecov Report

Merging #2811 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #2811      +/-   ##
============================================
+ Coverage     35.04%   35.05%   +0.01%     
  Complexity     2102     2102              
============================================
  Files           541      541              
  Lines         19322    19322              
  Branches       1829     1829              
============================================
+ Hits           6771     6774       +3     
+ Misses        11740    11738       -2     
+ Partials        811      810       -1

RingerJK

Please check dokka configuration, because packages were changed so need to change them at dokka config 👀

RingerJK · 2020-04-21T08:59:57Z

libnavigation-base/src/main/java/com/mapbox/navigation/base/trip/model/RouteProgress.kt

 * information.
 *
- * The latest route progress object can be obtained through the [RouteProgressObserver].
+ * The latest route progress object can be obtained through the [com.mapbox.navigation.base.trip.RouteProgressObserver.RouteProgressObserver].


typo

Suggested change

* The latest route progress object can be obtained through the [com.mapbox.navigation.base.trip.RouteProgressObserver.RouteProgressObserver].

* The latest route progress object can be obtained through the [com.mapbox.navigation.base.trip.RouteProgressObserver].

RingerJK · 2020-04-21T09:05:17Z

...ation-base/src/main/java/com/mapbox/navigation/base/internal/extensions/RouteOptionsUtils.kt

 @file:JvmName("MapboxRouteOptionsUtils")

-package com.mapbox.navigation.base.extensions
+package com.mapbox.navigation.base.internal.extensions


should fun RouteOptions.Builder.applyDefaultParams and fun RouteOptions.Builder.coordinates be internal?

Thought about this too and IMO everything should be private until proven otherwise 😅 In other words, it's better to keep it internal and if customers request this we can always make it public afterwards (but not the other way around as we'd break SemVer 😉).

it cannot be internal, examples uses it. The same as above

RingerJK · 2020-04-21T09:13:12Z

libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/extensions/LocaleEx.kt

@@ -1,4 +1,4 @@
-package com.mapbox.navigation.base.extensions
+package com.mapbox.navigation.base.internal.extensions


why do we need this internal?

Same as above. All these extensions are only used internally and I don't think they're that useful for developers as they can create their own. The less public APIs, the better. Otherwise we need to maintain them, we cannot change them without breaking SemVer, etc.

Guardiola31337 · 2020-04-21T13:42:43Z

Addressed feedback. This is ready for another round of 👀 @mapbox/navigation-android

RingerJK · 2020-04-21T15:04:32Z

libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/extensions/ContextEx.kt

 @file:JvmName("ContextEx")

-package com.mapbox.navigation.base.extensions
+package com.mapbox.navigation.base.internal.extensions


it cannot be internal, examples uses it

examples is our test app, we can use it 😉

… libdirections-onboard and libnavigation-base and bump dokka dependency version to 0.10.1

LukasPaczos · 2020-04-23T11:47:16Z

libnavigation-base/src/main/java/com/mapbox/navigation/base/trip/RouteProgressObserver.kt

@@ -1,4 +1,4 @@
-package com.mapbox.navigation.core.trip.session
+package com.mapbox.navigation.base.trip


Any particular reason this was exposed? The observer is still tied to the MapboxTripSession, which is internal. Is there a scenario where the observer would be used without using core?

Moved it so it lives near the RouteProgress and the rest of contracts and models (as it is one of the main observers accessed by most of the modules and needed publicly for clients). Although, that's a great point. Currently MapboxTripSession lives in libnavigation-core and I can't think of any use case in which you might want to use the observer alone. Can move it back to libnavigation-core as part of the missing internal package refactoring #2417 of libnavigation-core 👍

Guardiola31337 added ✓ ready for review Core Work related to core navigation and integrations. labels Apr 20, 2020

Guardiola31337 added this to the v1.0.0 milestone Apr 20, 2020

Guardiola31337 requested a review from a team April 20, 2020 22:43

Guardiola31337 self-assigned this Apr 20, 2020

Guardiola31337 added the backwards incompatible Requires a SEMVER major version change. label Apr 20, 2020

RingerJK reviewed Apr 21, 2020

View reviewed changes

Guardiola31337 force-pushed the pg-1.0-internal-refactor branch from bef249d to 3e11249 Compare April 21, 2020 13:41

Guardiola31337 requested a review from a team April 21, 2020 13:42

Guardiola31337 force-pushed the pg-1.0-internal-refactor branch 2 times, most recently from 9fe521c to 994c0bc Compare April 21, 2020 14:18

Guardiola31337 mentioned this pull request Apr 21, 2020

Junit framework is deprecated #2817

Merged

10 tasks

RingerJK reviewed Apr 21, 2020

View reviewed changes

add internal package to libdirections-hybrid, libdirections-offboard,…

7c9fa60

… libdirections-onboard and libnavigation-base and bump dokka dependency version to 0.10.1

Guardiola31337 force-pushed the pg-1.0-internal-refactor branch from 994c0bc to 7c9fa60 Compare April 21, 2020 15:26

Guardiola31337 requested a review from a team April 21, 2020 17:22

kmadsen approved these changes Apr 21, 2020

View reviewed changes

Guardiola31337 merged commit 0f7cfe8 into master Apr 21, 2020

Guardiola31337 deleted the pg-1.0-internal-refactor branch April 21, 2020 18:28

Guardiola31337 mentioned this pull request Apr 21, 2020

Add internal package to libnavigation-util #2820

Merged

9 tasks

JunDai mentioned this pull request Apr 21, 2020

fix current master conflict #2821

Merged

LukasPaczos reviewed Apr 23, 2020

View reviewed changes

This was referenced Apr 23, 2020

Add internal package to libnavigator and libtrip-notification #2836

Merged

Add internal package to libnavigation-core #2839

Merged

	* The latest route progress object can be obtained through the [com.mapbox.navigation.base.trip.RouteProgressObserver.RouteProgressObserver].
	* The latest route progress object can be obtained through the [com.mapbox.navigation.base.trip.RouteProgressObserver].

		@@ -1,4 +1,4 @@
		package com.mapbox.navigation.base.extensions
		package com.mapbox.navigation.base.internal.extensions

		@@ -1,4 +1,4 @@
		package com.mapbox.navigation.core.trip.session
		package com.mapbox.navigation.base.trip

Add internal package to 1.0 modules #2811

Add internal package to 1.0 modules #2811

Uh oh!

Conversation

Guardiola31337 commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Goal

Implementation

Testing

Checklist

Uh oh!

codecov-io commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RingerJK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RingerJK Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Guardiola31337 commented Apr 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Guardiola31337 commented Apr 20, 2020 •

edited

Loading

codecov-io commented Apr 20, 2020 •

edited

Loading

RingerJK Apr 21, 2020 •

edited

Loading