Refactor: Move classes to internal package

[abhishek1508]

Description

Move appropriate classes to internal package. (Fixes #2864)

• 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

Move appropriate classes to internal package.

Implementation

Move appropriate classes to internal package.

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

[codecov-commenter]

Codecov Report

Merging #3074 into master will increase coverage by 0.41%.
The diff coverage is 40.17%.

@@             Coverage Diff              @@
##             master    #3074      +/-   ##
============================================
+ Coverage     38.27%   38.68%   +0.41%     
  Complexity     2245     2245              
============================================
  Files           546      542       -4     
  Lines         19765    19596     -169     
  Branches       1871     1850      -21     
============================================
+ Hits           7565     7581      +16     
+ Misses        11321    11137     -184     
+ Partials        879      878       -1     

[JunDai]

@abhishek1508 - qq: what's the purpose of moving the class into internal package?
I was thinking it's to restrict the access within its module, but seems it's not?

[abhishek1508]

qq: what's the purpose of moving the class into internal package?
I was thinking it's to restrict the access within its module, but seems it's not?

There are public classes, that have been written so that they can be used across different packages. But, these classes are not meant to be used by external developers or rather they should not depend on it. Moving it to internal gives them the impression that these public classes are only meant to be changed internally by the SDK and can introduce any breaking changes. They should only rely on the public API's exposed. The same approach is followed by Square
cc @JunDai

[Guardiola31337]

Good starting @abhishek1508

Left some comments.

[Guardiola31337]

Some comments / questions 👇 (we don't need to address all of these as part of these PR but wanted to keep track of them)

• We're mixing Java and Kotlin docs, could that be an issue? cc @langsmith
• Does FeedbackSubTypeAdapter need the internal constructor if the class is internal?
• Does TurnLaneAdapter need to be public?
• Does BannerInstructionModel need to be public?
• Unnecessary public modifiers in BannerShield
• Unnecessary public modifiers in ImageCreator
• Why is TurnLaneView internal?
• Why all the listeners under internal.listeners are internal?
• Some constants in RouteConstants are never used VANISHING_ROUTE_LINE_UPDATE_DELAY for instance
• Does CompareUtils need to be under internal? It has the internal modifier already
  This is the only kotlin file in Utils. Doesn't make sense to have another util package outside internal just to host 1 file. Will let it be inside internal
• Should MapUtils#addLayerToMap be removed for 1.0? It's @Deprecated and internal
• Does MobileNetworkChecker need to be under internal? It is internal already
  Yes. It is package private. But ConnectivityStatusProvider is public and lives inside internal package. ConnectivityStatusProvider uses MobileNetworkChecker and hence it needs to be live in the same package as ConnectivityStatusProvider or be made public.
• Does WifiNetworkChecker need to be under internal? It is internal already
  Yes. It is package private. But ConnectivityStatusProvider is public and lives inside internal package. ConnectivityStatusProvider uses WifiNetworkChecker and hence it needs to be live in the same package as ConnectivityStatusProvider or be made public.
• Could we remove the legacy package? Moving out the classes in there into other modules
• Some constants in NavigationConstants are never used
• Does DefaultMapboxPuckDrawableSupplier need to be public?
• Does NavigationPuckPresenter need to be public?
• Does the NavigationViewModel need to be public?
  Moving this to internal will result in a whole lot files being changed. It is not worth going through that amount of changes. Will leave it as is.

[Guardiola31337]

@abhishek1508

This is the only kotlin file in Utils. Doesn't make sense to have another util package outside internal just to host 1 file. Will let it be inside internal

We are not forced to create a package 😅 Pointed it out to be consistent across the codebase.

• Does DefaultMapboxPuckDrawableSupplier need to be public?
• Does NavigationPuckPresenter need to be public?

Maybe now that both DefaultMapboxPuckDrawableSupplier and NavigationPuckPresenter are package-private we can extract puck package out of the internal

• Does MobileNetworkChecker need to be under internal? It is internal already
  Yes. It is package private. But ConnectivityStatusProvider is public and lives inside internal package. ConnectivityStatusProvider uses MobileNetworkChecker and hence it needs to be live in the same package as ConnectivityStatusProvider or be made public.
• Does WifiNetworkChecker need to be under internal? It is internal already
  Yes. It is package private. But ConnectivityStatusProvider is public and lives inside internal package. ConnectivityStatusProvider uses WifiNetworkChecker and hence it needs to be live in the same package as ConnectivityStatusProvider or be made public.

👍

• Does the NavigationViewModel need to be public?
  Moving this to internal will result in a whole lot files being changed. It is not worth going through that amount of changes. Will leave it as is.

👍

• Why all the listeners under internal.listeners are internal?

These seem that need to be exposed publicly. Am I missing something?