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

Migrate to RxJava 3 #441

Merged
merged 1 commit into from Jun 3, 2020
Merged

Conversation

MrAsterisco
Copy link

This PR introduces the following update: support for RxJava 3 (potentially closing #365 ).

I wanted to use ReactiveNetwork in a new project that targets RxJava 3, so I figured I might as well make the transition myself.

A few major things have changed:

  • I have changed the package name to "rx3".
  • The Build Tools are now on "29.0.2" to support the new Gradle plugin in Android Studio 4.
  • Gradle version is now at "6.3".
  • The onError method of the NetworkObservingStrategy interface is now using a Throwable instead of a simple Exception. This had to be changed because a lot of methods inside the various implementations of the interface can throw a generic Throwable.
  • The android.enableUnitTestBinaryResources has been removed because it is no longer supported.

The rest of the changes are just related to the new version of RxJava (mainly changing the package references).

All unit tests pass, all code styles checks pass. Both example apps work perfectly (I had to add the Java compatibility option to the Kotlin app to make it build). The build on Travis works.

Please, let me know if you're interested in merging this PR and, if so, what would be the next steps.

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2020

Codecov Report

Merging #441 into RxJava3.x will increase coverage by 0.22%.
The diff coverage is 77.14%.

Impacted file tree graph

@@               Coverage Diff               @@
##             RxJava3.x     #441      +/-   ##
===============================================
+ Coverage        83.36%   83.59%   +0.22%     
- Complexity         118      131      +13     
===============================================
  Files               11       11              
  Lines              457      451       -6     
  Branches            38       36       -2     
===============================================
- Hits               381      377       -4     
+ Misses              51       49       -2     
  Partials            25       25              
Impacted Files Coverage Δ Complexity Δ
...hen/reactivenetwork/library/rx3/Preconditions.java 86.66% <ø> (ø) 12.00 <0.00> (?)
...n/reactivenetwork/library/rx3/ReactiveNetwork.java 83.33% <ø> (ø) 11.00 <0.00> (?)
.../internet/observing/error/DefaultErrorHandler.java 100.00% <ø> (ø) 2.00 <0.00> (?)
.../strategy/PreLollipopNetworkObservingStrategy.java 63.33% <63.33%> (ø) 8.00 <8.00> (?)
...chen/reactivenetwork/library/rx3/Connectivity.java 68.14% <66.66%> (ø) 24.00 <12.00> (?)
.../strategy/MarshmallowNetworkObservingStrategy.java 98.38% <90.00%> (ø) 22.00 <1.00> (?)
...tivenetwork/library/rx3/ConnectivityPredicate.java 100.00% <100.00%> (ø) 7.00 <4.00> (?)
.../internet/observing/InternetObservingSettings.java 96.15% <100.00%> (ø) 12.00 <1.00> (?)
...ving/strategy/SocketInternetObservingStrategy.java 94.11% <100.00%> (ø) 11.00 <3.00> (?)
...trategy/WalledGardenInternetObservingStrategy.java 95.91% <100.00%> (ø) 15.00 <3.00> (?)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58b0b17...15173fe. Read the comment docs.

@pwittchen
Copy link
Owner

Hi @MrAsterisco,

Thanks for this PR!

Could you please squash your commits into one and set target branch to RxJava3.x? I know it didn't exist before, but I just created it and I'd like to have changes related to RxJava3.x on that branch and RxJava2.x should contain code depending on RxJava2.x only.

Regards,
Piotr

@pwittchen pwittchen changed the base branch from RxJava2.x to RxJava3.x June 1, 2020 14:51
@pwittchen
Copy link
Owner

I already changed target branch (base) to RxJava3.x. I found out it was possible in this PR.

@MrAsterisco
Copy link
Author

Ok, great! Before squashing my commits, would you like me to go through the process of updating the Changelog file and generating the new documentation?

@pwittchen
Copy link
Owner

pwittchen commented Jun 1, 2020

You can update changelog and documentation in README.md file. I'll need to update JavaDocs and website on gh-pages later separately because that's another branch.

@MrAsterisco
Copy link
Author

Ok, done! I'm assuming that this will be a 4.0.0, since there is a (small) breaking change in the NetworkObservingStrategy interface.

Copy link
Owner

@pwittchen pwittchen left a comment

Choose a reason for hiding this comment

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

Changes look fine. I have a few issues with the documentation in README.md. Moreover, I have a question: Did you use code formatter / code style described here: https://github.com/pwittchen/ReactiveNetwork#code-style ? If not, then please apply it and reformat code with it.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@MrAsterisco MrAsterisco force-pushed the RxJava3.x branch 2 times, most recently from 2cc2a09 to e18e76a Compare June 3, 2020 07:58
@MrAsterisco
Copy link
Author

OK! I've reformatted the code using the AndroidSquare style (about 30 files were not formatted correctly) and I've also updated the README as you asked (I've also changed the references to the RxJava2.x and the name of the artifact).

I noticed there's a Contributors section, so I've added myself in there; hope that's ok.

@pwittchen
Copy link
Owner

pwittchen commented Jun 3, 2020

Looks good. There's one more change required for migration to the new RxJava version. In the library/gradle.properties file, we need to change:

POM_ARTIFACT_ID=reactivenetwork-rx2

to:

POM_ARTIFACT_ID=reactivenetwork-rx3

Fix code style

Update README and CHANGELOG

Apply code style, update README

Update artifact name

Update artifact name, proguard rules, version and version code
@MrAsterisco
Copy link
Author

Done! To be sure, I've also searched for "rx2" in the whole project and I've updated the proguard rules files to point to the correct package.

I've also updated VERSION_NAME and VERSION_CODE in gradle.properties.

@pwittchen pwittchen merged commit 808b7f5 into pwittchen:RxJava3.x Jun 3, 2020
@pwittchen
Copy link
Owner

Merged. It will be available in the next release for RxJava3.x. Thanks for your contribution @MrAsterisco!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants