search

commons-app / apps-android-commons

The Wikimedia Commons Android app allows users to upload pictures from their Android phone/tablet to Wikimedia Commons
https://commons-app.github.io/
Apache License 2.0
982 stars 1.18k forks source link

RxJava2 is "end of life" #4665

Open psh opened 2 years ago

psh commented 2 years ago

Summary:

The authors of RxJava put RxJava2 into "maintenance mode" when they released RxJava3 and have since stated

The 2.x version is end-of-life as of February 28, 2021. No further development, support, maintenance, PRs and updates will happen. The Javadoc of the very last version, 2.2.21, will remain accessible.

They have written up a lengthy "What's different in 3.0" (see: https://github.com/ReactiveX/RxJava/wiki/What's-different-in-3.0)

The mix of Java and Kotlin in this project suggest that the migration path forward would be to look at RxJava3 but that ought to be balanced by Google's official statement that

Coroutines is our recommended solution for asynchronous programming on Android.

Would you like to work on the issue?

Happy to help out, whichever way the project goes.

misaochan commented 2 years ago

@ashishkumar468 @madhurgupta10 What do you think?

madhurgupta10 commented 2 years ago

IMO, going directly to Coroutines and Flows would be a lot better, not sure about MVP but for MVVM this is a recommended solution :)

rohit9625 commented 4 months ago

Right, using Flows and Coroutines might also improve the app's performance. And it's super easy to work on and maintain :)

psh commented 4 months ago

There are 5 or so Java classes that use our Retrofit interfaces

The rest of them are already Kotlin. I have improved unit tests and the Kotlin conversion of those staged on a couple of branches ready to submit as PRs.

With all the Retrofit interfaces consistently in Kotlin already, and their API Client classes also Kotlin we would be in a place to swap RX Single and Observable out of the API layer, and replace with simple suspend methods on the API interface classes. Jetbrains already provide a bridging library kotlinx-coroutines-rx2 that makes it a breeze to not have to change the entire application in 1 go.

neeldoshii commented 4 months ago

There are 5 or so Java classes that use our Retrofit interfaces

  • UserClient
  • WikidataClient
  • WikiBaseClient
  • ReviewHelper
  • UploadClient

The rest of them are already Kotlin. I have improved unit tests and the Kotlin conversion of those staged on a couple of branches ready to submit as PRs.

With all the Retrofit interfaces consistently in Kotlin already, and their API Client classes also Kotlin we would be in a place to swap RX Single and Observable out of the API layer, and replace with simple suspend methods on the API interface classes. Jetbrains already provide a bridging library kotlinx-coroutines-rx2 that makes it a breeze to not have to change the entire application in 1 go.

Is there any way I can be in use helping in migrating over here @psh? Would be happy to contribute to it 😊

nicolas-raoul commented 3 months ago

Any estimate of how many hours of work it would take an average developer to upgrade all? :-)

psh commented 3 months ago

I did some very general poking around to start to scope this out.

Language breakdown

Coroutines are a Kotlin technology. There's great integration into the Android SDK for lifecycle aware coroutine contexts, but it will mean migrating a body of the Java code to Kotlin.

Counting non-blank lines of code in various files - easy to sum in a spreadsheet afterwards

for i in `find . -name \*.java`; do
echo $i, `cat $i | grep '[^ ]' | wc -l`
done

Overall, this amounts to roughly 40,000 lines of Java and 35,000 lines of Kotlin in the project. We wouldn't need to convert everything though! 😺

Remove RxBinding library

We could break off one smaller migration effort (removing RxBinding) that could treat the Kotlin migration as optional if you skip feeding things into a Flow

Where are we using some of the RxJava classes today?

Obviously there is overlap between things; files may contain multiple references to Single, Observable, etc. Nuances to watch out for - how often do we cancel / dispose something we subscribe to? Where are would we get a coroutine context from, and will that mean moving the conversion up further into the UI (notably, lifecycle awareness)

Useful / interesting resources

rohit9625 commented 3 months ago

Hello @psh Is this a GSoc-worthy project? If yes, then I would like to include it in my GSoc proposal. And if not then I still want to work on this migration :)

neeldoshii commented 3 months ago

Hello @psh

Is this a GSoc-worthy project? If yes, then I would like to include it in my GSoc proposal. And if not then I still want to work on this migration :)

+1 Even I was thinking of asking the same thing. Now that my account is unbanned I can start working on this too.

nicolas-raoul commented 3 months ago

@psh Thanks a lot for the great details of what needs to be done! Any ballpark figure of how many hours it would take an average developer (meaning not you 😉)?

psh commented 2 months ago

As food for thought, I created a PR where I did a sample migration. Taking login as a candidate (since there's only 3 API calls, but plenty of Java and other code) I worked through the migration process of adopting coroutines. See the PR - https://github.com/commons-app/apps-android-commons/pull/5695

What I tried to do was lay a foundation in the PR to aid a migration while also working through the process.

shashankiitbhu commented 2 months ago

@psh Interesting, I am looking at this PR, this provides a good structure for the migration