Update to RxJava2 #239
Update to RxJava2 #239
Conversation
| try { | ||
| disposeAction.run(); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); // RxJava2 Action throws an exception. Pass it on |
There was a problem hiding this comment.
Should we rethrow here or use the RxJavaPlugins?
There was a problem hiding this comment.
Please check out the updated implementation which converts FinishAsyncJobSubscription into FinishAsyncJobAction and passes it through to Disposables.fromAction(finishAsyncJobAction)
| import rx.functions.Action0; | ||
|
|
||
| public class DisposableSubscription implements Subscription { | ||
| public class DisposableSubscription implements Disposable { |
There was a problem hiding this comment.
I feel like this could be solved with Disposables.from instead
dependencies.gradle
Outdated
| rxLint : '1.6', | ||
| supportLibs : '23.1.1', | ||
| okHttp : '3.2.0', | ||
| retrofit : '2.0.0-beta4', | ||
| okHttp : '3.8.0', |
|
Updated based on review. Not sure why the travis job is failing, but it seems to be failing for all recent PRs. Is this something I can fix in this PR? |
|
Sorry, I'm at KotlinConf and then will spend weekend with friends who also came to conference and then DroidconSF and work… I've added this to my todo list and hopefully will review in the middle of next week, apologies 😬 I really trust @vanniktech, so you can definitely continue addressing his comments 😸 |
|
I don't think that we need the |
|
|
@danielrees18 is this on the latest master branch? If not, rebasing should fix the build issue. |
|
Haven't kept up with changes master, I'll do that shortly
…On Dec 26, 2017 6:38 PM, "Niklas Baudy" ***@***.***> wrote:
@danielrees18 <https://github.com/danielrees18> is this on the latest
master branch? If not, rebasing should fix the build issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#239 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACwONw8MaRNCr0XhiVRBjczjebl6pKrLks5tEYNegaJpZM4QQuw_>
.
|
There was a problem hiding this comment.
Looks good on my side.
As for the ChangeableBaseUrl, I'll leave it up to @artem-zinnatullin how / if that class needs a change.
|
is there any updates on this PR, interested to see Rxjava2 in this project to experiment with it. |
|
ProGuard config needs an update, build fails https://api.travis-ci.org/v3/job/323862434/log.txt |
Closes #208 and updates the code to use
io.reactivexclasses instead ofrxclasses. This PR also makes #223 obsolete. Also updated the Retrofit2 RxJava adapter to the RxJava2 adapter.In the process I also updated Retrofit from
2.0.0-beta4to2.3.0andOkHttpfrom3.5.0to3.8.0. This had some repercussions withChangeableBaseUrlwhich I believe I resolved correctly but please check. This would also make #236 obsolete though that PR overhaulsChangeableBaseUrland upgrades OkHttp to 3.9.0 while Retrofit2.3.0is using OkHttp3.8.0under the hood.All tests are ✅