RxJava1 bindings #341
RxJava1 bindings #341
Conversation
|
This looks like a good approach to me 👍 |
apollo-runtime/build.gradle
Outdated
| @@ -36,6 +36,7 @@ dependencies { | |||
| compile dep.supportAnnotations | |||
| compile dep.okHttp | |||
| compile dep.moshi | |||
| compile dep.rxjava | |||
There was a problem hiding this comment.
shouldn't it be in separate module?
There was a problem hiding this comment.
yup! will move to module next, wanted to get feedback on api being ok
| @@ -0,0 +1,74 @@ | |||
| package com.apollographql.android.impl; | |||
There was a problem hiding this comment.
I guess this class must be in :apollo-rxsupport
| } | ||
|
|
||
|
|
||
| public static <T> Observable<T> from(final ApolloCall<T> call) { |
| return; | ||
| } | ||
| if (!subscriber.isUnsubscribed()) { | ||
| subscriber.onCompleted(); |
There was a problem hiding this comment.
is onError not enough? I thought you must call onError or onCompleted but not both
There was a problem hiding this comment.
I copied from retrofit, its returning from catch block so it will on complete if no exception
| } | ||
|
|
||
| public static <T> Single<T> from(final ApolloCall<T> call) { | ||
| return Single.create(new Single.OnSubscribe<T>() { |
There was a problem hiding this comment.
if you change it to Single.fromCallable then you don't need try/catch
There was a problem hiding this comment.
Oh you need access to subscriber
| import rx.functions.Action0; | ||
| import rx.subscriptions.Subscriptions; | ||
|
|
||
| public class RxApollo { |
There was a problem hiding this comment.
final, with private constructor?
|
so fyi rx-support module needs to be android library since it needs |
digitalbuddha
added
Priority: Medium
✨ Type: Feature
🚗 Runtime
and removed
Status: DO NOT MERGE
labels
|
closes #243 |
| } | ||
| } | ||
|
|
||
| apply plugin: 'com.android.library' |
There was a problem hiding this comment.
you think it should be android lib?
There was a problem hiding this comment.
it depends on Runtime which is currently an android lib. If/when we move sql stuff out to its own module we can change this to java
apollo-rxsupport/gradle.properties
Outdated
| @@ -0,0 +1,4 @@ | |||
| POM_ARTIFACT_ID=rx | |||
| POM_NAME=ApolloStack Runtime | |||
apollo-rxsupport/gradle.properties
Outdated
| @@ -0,0 +1,4 @@ | |||
| POM_ARTIFACT_ID=rx | |||
| @@ -10,7 +10,8 @@ | |||
| import javax.annotation.Nonnull; | |||
| import javax.annotation.Nullable; | |||
|
|
|||
| public interface ApolloCall<T> { | |||
|
|
|||
apollo-rxsupport/build.gradle
Outdated
| compile dep.moshi | ||
| compile dep.rxjava | ||
| compile project(":apollo-runtime") | ||
| compile project(":apollo-api") |
There was a problem hiding this comment.
both of those should be provided because rx-support should assume those would be available at runtime
(Also, compile project(":apollo-runtime") doesn't bring the transitive compile project deps with it. That's why having compile project(":apollo-api") under runtime, still required you to bring that in.
apollo-integration/build.gradle
Outdated
| @@ -26,8 +26,9 @@ android { | |||
| } | |||
|
|
|||
| dependencies { | |||
| provided project(':apollo-api') | |||
| compile project(':apollo-api') | |||
There was a problem hiding this comment.
can be provided because otherwise it would cause dex errors when the plugin is added.
apollo-rxsupport/build.gradle
Outdated
| dependencies { | ||
| compile dep.jsr305 | ||
| compile dep.supportAnnotations | ||
| compile dep.okHttp |
There was a problem hiding this comment.
ditto all those apart from Rx should be provided since runtime would bring those already.
There was a problem hiding this comment.
actually, I don't think you'd need to include any of those. apart from Rx.
I wanted to gather thoughts on how the Rx stuff should work. Here's an attempt at doing wrappers for
ApolloWatcher&ApolloCall. Similar to Retrofit we will cancel a request when theObservableis unsubscribed from.