mohaxspb / ScpFoundationCore

DEPRECATED. SCP Foundation sites offline reader for Android
https://scpfoundation.app/?utm_source=github.com&utm_medium=directLink&utm_campaign=repo_website
16 stars 3 forks source link

some code cleanup #196

Closed TacoTheDank closed 5 years ago

TacoTheDank commented 5 years ago

also reformatted code (second "app" commit should be named "core", oops)

i would suggest taking the next couple of days to update glide to 4.x and the reactivex libraries to 2.x, as they have a lot of performance improvements that can be utilized

mohaxspb commented 5 years ago

Hello!

Thanks for contributing! =) But now I can't merge this PR as there are some problems.

  1. Some libs do not need updates. I.e. firebase ones. There is a bug with version updates suggestions in Android Studio. It suggests 16.0.5 for some dependecies, but there are no artifacts in maven for these version. You can find correct latest firebase versions here: https://firebase.google.com/support/release-notes/android
  2. Some libs come with appodeal lib (ogury, adcolony, facebook). So I prefer use older versions, as they must work with appodeal and there is no guarantee, that it will work with latest ones.
  3. With butterknife I suffered compile problems with latest version some times ago, so no need to update it now.
  4. I prefer $daggerVersion vs ${daggerVersion}. I think we do not need {} here.
  5. PR is too large. It's really hard to review 4000+ lines of changes in 282 files). Please, create a smaller PR's. I.e. ~10 files per one.
  6. I really need to crash gradle build if there is no config files as I build APKs on CI and I need messages in logs if I forgot to add them to server.

Updating Glide is good idea. But there can be some problems, as 4+ version uses code generation and can require some settings to work with current gradle modules architecture.

But RxJava can't be updated simply to 2+, as current Realm version requires RxJava:1+ and if we update it to 2+ we also need to update Realm with checks that it will not corrupt users DB. Also almost all code in ApiClient and DbProvider uses RxJava:1+ code, which requires total refactoring in this case and can produce a lot of problems. In future (may be at summer) I plan to rewrite app with use of Room and at this point update of RxJava can be done)

mohaxspb commented 5 years ago

Also com.github.jakob-grabner:Circle-Progress-View:v1.3 cannot be updated, as 1.4 version has 19 as minSdkVersion, and app uses 17

mohaxspb commented 5 years ago

Also in some libs in latest versions authors use libs from androidx package, which causes a lot of conflicts. So we need to migrate whole project to androidx, but it's huge task and maybe cant be done now, as not all dependencies uses androidx. So, we may try to do it later, i.e. at summer.

TacoTheDank commented 5 years ago

With firebase and other such libraries, Google doesn't always update the listing on those sites. This is the actual maven repository (that they're downloaded from) where you can check the latest versions https://dl.google.com/dl/android/maven2/index.html (you can bookmark it)

When I put // and a version number next to a thing, that just means there's a new version available (not that you have to use it :))

And yeah, updating Glide and Reactivex are gonna be huge tasks that will take a long time (respectively why I mentioned them), and of course there will be lots of breaking changes (and there WILL be problems haha), but I believe you can and will be able to fix them.

Also, you might find https://github.com/akarnokd/RxJava2Interop useful; it's a library "to convert between RxJava 1.x and 2.x reactive types."

mohaxspb commented 5 years ago

@TacoTheDank I tryed to update firebase libs to latest versions and failed to compile. So I can't merge this commit(

Also I can't find time to review 4000+ lines of changes. So, If you want to continue contributing, please, create smaller PR and check if code compiles.

TacoTheDank commented 5 years ago

alrighty