Skip to content
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

Support RxJava #131

Closed
zugaldia opened this issue Jul 14, 2016 · 9 comments
Closed

Support RxJava #131

zugaldia opened this issue Jul 14, 2016 · 9 comments

Comments

@zugaldia
Copy link
Member

/cc: @cammace @bleege

@bleege
Copy link
Contributor

bleege commented Jul 14, 2016

@zugaldia Can you provide a little more detail as to how this will work as well as to reassure people publicly that RxJava support is still going to be available? Thanks!

@zugaldia
Copy link
Member Author

zugaldia commented Aug 4, 2016

@bleege We're still trying to figure out the best plan of action here as there're a few different approaches that we can try (and we're open for feedback). Let's discuss this together.

In the meanwhile, I'm gonna remove RxJava support from master, and therefore from the SNAPSHOT. We'll provide an upgrade path for RxJava support before the next MAS release is published.

/cc: @ivovandongen

@zugaldia
Copy link
Member Author

zugaldia commented Aug 5, 2016

Just reopening until we decide on path forward for RxJava support.

@zugaldia zugaldia reopened this Aug 5, 2016
@bleege
Copy link
Contributor

bleege commented Aug 9, 2016

@zugaldia Awesome! Just wanted to make sure implementing developers knew that they weren't being abandoned. Thanks!

@zugaldia
Copy link
Member Author

We currently ship two different products as part of this project:

  1. libjava aka mapbox-java-services (pure Java, no Android dependencies)
  2. libandroid aka mapbox-android-services (Android dependencies).

With the change required by this ticket to extract RxJava from libjava we need to create a third package (pure Java too) providing the RxJava extensions. That'd be 3 packages, and I think it's one too many. I'd like to propose a different approach.

We make this repo and its products Java-only (as the name actually indicates) with no Android dependencies. It'd have two products:

  1. libjava aka mapbox-java-services as it's today (pure Java, no Android dependencies)
  2. (new) libjava-rx aka mapbox-java-rxjava-services (pure Java too, RxJava extensions)

The remaining code in this repo, the Android-specific code, is pretty minimal actually: one object replacing Android's Geocoder object, one geocoder autocomplete widget, and an utils file for permissions. Considering that libjava is now part of the map SDK (aka mapbox-gl-native) due to mapbox/mapbox-gl-native#5869 these classes could be easily moved to the map SDK with minimal impact (I suggest a services packages containing this functionality).

The end result would be that all Android-specific code would live in mapbox-gl-native while all pure Java code lives here.

@mapbox/android I'd love to hear your thoughts. Please 👍 this comment if this looks like a plan or leave a comment otherwise.

@ivovandongen
Copy link
Contributor

I'd also suggest to make those pure java modules pure java. So no android manifest and maybe even maven instead of gradle? Makes it more flexible in my opinion.

@bleege
Copy link
Contributor

bleege commented Aug 23, 2016

I'd also suggest to make those pure java modules pure java. So no android manifest and maybe even maven instead of gradle?

@ivovandongen's on the right path with this IMHO. If it's a Java repo, it's 💯 pure Java. I'd suggest that we stick with Gradle though as it's what is used already on the other projects and there's no need to introduce yet another build system into the matrix.

Considering that libjava is now part of the map SDK (aka mapbox-gl-native) due to mapbox/mapbox-gl-native#5869 these classes could be easily moved to the map SDK with minimal impact (I suggest a services packages containing this functionality).

The only question I have about this is doesn't this prevent developers from using Mapbox Services independently from having to also use the much larger SDK or is this not a business concern anymore?

@ivovandongen
Copy link
Contributor

I'd suggest that we stick with Gradle though as it's what is used already on the other projects and there's no need to introduce yet another build system into the matrix.

Agreed, if gradle can handle optional dependencies nicely.

@bleege
Copy link
Contributor

bleege commented Aug 23, 2016

The only question I have about this is doesn't this prevent developers from using Mapbox Services independently from having to also use the much larger SDK or is this not a business concern anymore?

Following up on this, @zugaldia clarified this for me in Slack a few minutes ago and confirmed that it's his intentions for libjava is now part of the map SDK to be done only as an external dependency in the SDK's Gradle file and not at the source code level. This means that MAS will remain as an independent library and usable sans the SDK as it always has been. 👍

Agreed, if gradle can handle optional dependencies nicely.

Yep, looks like Gradle just added this as of v2.12 so this should work out fine as Google is strongly recommending to use v2.14.1 at a minimum due to a security bug.

https://gradle.org/blog/compile-only-dependencies/

@cammace cammace changed the title Extract RxJava to reduce method count Support RxJava Dec 6, 2016
@zugaldia zugaldia mentioned this issue Feb 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants