-
Notifications
You must be signed in to change notification settings - Fork 8
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
Align module generation and skip configuration by default #26
Conversation
/** | ||
* Exposes logger hooks. | ||
*/ | ||
CommonLogger("Logger", "com.mapbox.base.common.logger", "Logger"), |
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.
This takes #21 into consideration already @Lebedinsky. Depending on which is merged first, we can resolve the conflicts.
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.
One downside is, that all params providers will need to overwrite all module types
I'm fine with this as a drawback, maybe some kind of adapter approach would work here.
annotations/src/main/java/com/mapbox/annotation/module/MapboxModuleType.kt
Show resolved
Hide resolved
common/src/main/java/com/mapbox/common/module/provider/MapboxModuleProvider.kt
Show resolved
Hide resolved
Ready for another round. |
I added a bunch more of sanity tests, example usage in the docs and cleaned up the code. |
common/src/main/java/com/mapbox/common/module/provider/MapboxModuleProvider.kt
Outdated
Show resolved
Hide resolved
val provider = providerField.get(null) | ||
|
||
if (provider != null) { | ||
// get module instance from the provider |
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.
every time you feel the need to write a comment, you can consider writing a function instead
common/src/main/java/com/mapbox/common/module/provider/MapboxModuleProvider.kt
Outdated
Show resolved
Hide resolved
acaae3a
to
dd99a9d
Compare
Closes #14. Closes #18.
This PR consolidates module generation from both Maps and Nav SDKs and ships the provider in the
common
library.This removes the separation of the module types - now all of them are under one
enum
constant. This not only helps to make the provider generic but also will allow us to write simpler docs.One downside is, that all params providers will need to overwrite all module types, even if particular SDK is not supporting them. For example, Navigation SDKs param provider will look something like:
however, we'll never hit the
else
branch, unless we make the mistake ourselves and it should be caught quickly.