Skip to content
This repository has been archived by the owner on Feb 4, 2024. It is now read-only.

feat: Migrate to RxJava 3 and RxBinding 4 fixes #317 #326

Merged
merged 1 commit into from
Jul 12, 2020

Conversation

galuszkak
Copy link
Collaborator

No description provided.

@galuszkak
Copy link
Collaborator Author

galuszkak commented Jul 10, 2020

Hi @tbruyelle and @epool ,

I've prepared here migration to RxJava 3 and move sample to use RxBinding 4 that also uses RxJava 3.

Tests are passing so I think this PR should be OK. This should fix #317

Thanks!

@galuszkak galuszkak changed the title feat: Migrate to RxJava 3 and RxBinding 4 feat: Migrate to RxJava 3 and RxBinding 4 Fixes #317 Jul 10, 2020
@galuszkak galuszkak changed the title feat: Migrate to RxJava 3 and RxBinding 4 Fixes #317 feat: Migrate to RxJava 3 and RxBinding 4 fixes #317 Jul 10, 2020
@tbruyelle
Copy link
Owner

Looks cleaner than #320 which embeds more changes. Good to me, WDYT @epool ?

@tbruyelle tbruyelle requested a review from epool July 10, 2020 07:54
Copy link
Collaborator

@epool epool left a comment

Choose a reason for hiding this comment

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

is this backward compatible with apps still using RxJava2?

@@ -513,7 +488,6 @@ public void eachSubscriptionCombined_trigger_granted() {
mRxPermissions.onRequestPermissionsResult(new String[]{permission}, result);

sub.assertNoErrors();
sub.assertNotTerminated();
Copy link
Collaborator

Choose a reason for hiding this comment

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

why these deletions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @epool ,

Those methods were removed in RxJava 3 from TestObservable. See here:
https://github.com/ReactiveX/RxJava/wiki/What's-different-in-3.0#test-support-methods

Copy link
Collaborator

Choose a reason for hiding this comment

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

got it, thanks!

@galuszkak
Copy link
Collaborator Author

@epool answering your question on compatibility with RxJava 2 if anyone would need their Observable to be V2 not V3 they can use bridge library called RxJavaBridge which is available here:
https://github.com/akarnokd/RxJavaBridge

@tbruyelle
Copy link
Owner

Thx @epool for the review
Thx @galuszkak for the clear PR !

@tbruyelle tbruyelle merged commit 405050e into tbruyelle:master Jul 12, 2020
@tbruyelle tbruyelle mentioned this pull request Jul 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants