Skip to content

Conversation

@Guardiola31337
Copy link
Contributor

@Guardiola31337 Guardiola31337 commented Apr 21, 2020

Description

This PR adds internal package to libnavigation-util

Splitting #2417 in different PRs

Follow up from #2811

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

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

@Guardiola31337 Guardiola31337 added ✓ ready for review backwards incompatible Requires a SEMVER major version change. Core Work related to core navigation and integrations. labels Apr 21, 2020
@Guardiola31337 Guardiola31337 added this to the v1.0.0 milestone Apr 21, 2020
@Guardiola31337 Guardiola31337 requested a review from a team April 21, 2020 21:02
@Guardiola31337 Guardiola31337 self-assigned this Apr 21, 2020
@Guardiola31337 Guardiola31337 requested a review from a team April 21, 2020 22:07
@Guardiola31337 Guardiola31337 force-pushed the pg-1.0-internal-refactor-utils branch from 2874a7c to d78a836 Compare April 21, 2020 22:10
package com.mapbox.navigation.trip.notification.utils.time.span

internal interface SpanItem {

Copy link
Contributor

Choose a reason for hiding this comment

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

want this space?

@Guardiola31337 Guardiola31337 force-pushed the pg-1.0-internal-refactor-utils branch 3 times, most recently from 8423f0e to 919b8a0 Compare April 22, 2020 00:04
@Guardiola31337 Guardiola31337 force-pushed the pg-1.0-internal-refactor-utils branch from 919b8a0 to 1418ecf Compare April 22, 2020 00:27
Comment on lines +558 to +560
Logger::class.java to MapboxModuleProvider.createModule(
MapboxModuleType.CommonLogger,
::paramsProvider
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regression from #2774 cc @Lebedinsky @LukasPaczos

NavigationOptions::class.java to navigationOptions
)
MapboxModuleType.CommonLogger -> arrayOf()
MapboxModuleType.CommonLogger -> arrayOf(Logger::class.java to logger)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regression from #2774 cc @Lebedinsky @LukasPaczos

@Guardiola31337 Guardiola31337 merged commit a27f0d7 into master Apr 22, 2020
@Guardiola31337 Guardiola31337 deleted the pg-1.0-internal-refactor-utils branch April 22, 2020 01:04
MapboxOnboardRouterConfig::class.java to (navigationOptions.onboardRouterConfig
?: throw RuntimeException(MAPBOX_NAVIGATION_TOKEN_EXCEPTION_ONBOARD_ROUTER))
?: throw RuntimeException(MAPBOX_NAVIGATION_TOKEN_EXCEPTION_ONBOARD_ROUTER)),
Logger::class.java to MapboxModuleProvider.createModule(

Choose a reason for hiding this comment

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

this could be Logger::class.java to logger

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

Labels

backwards incompatible Requires a SEMVER major version change. Core Work related to core navigation and integrations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants