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

Update most dependencies #4768

Merged
merged 3 commits into from
Nov 22, 2020
Merged

Update most dependencies #4768

merged 3 commits into from
Nov 22, 2020

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Nov 2, 2020

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This is basically #4508 but also contains:

  • further update androidx.constraintlayout:constraintlayout from 2.0.2 to 2.0.4
  • re-enable ktlint

Other than that, it is identical to #4508 (by @wb9688, he told us we could continue his PRs):

  • Update all dependencies except ExoPlayer. Most changes come from upgrading to RxJava 3, which unfortunately requires upgrading to an alpha version of Room, and Ktlint rules changes.
  • Note how I've added 3 dependencies: androidx.documentfile:documentfile and androidx.localbroadcastmanager:localbroadcastmanager were previously part of some other AndroidX library, while com.squareup.leakcanary:plumber-android is a new feature from LeakCanary that fixes known memory leaks in the Android framework.
  • I did not upgrade ExoPlayer, as we probably want to use certain new features like audio offloading, Media2 (replacement for MediaSessionCompat, which is now deprecated) support, and maybe toplevel playlist support. @Redirion: Do you want to do that in some PR? Also see Update most dependencies #4508 (comment)

APK testing

I tested a little bit on 4.4 but didn't have much time for further testing. @Poolitzer @mhmdanas @mqus @MD77MD @opusforlife2 could you test this, if you have some time? You'd have to just go through the screens and the buttons the app has and verify nothing crashes.
app-debug.zip

Due diligence

@triallax
Copy link
Contributor

triallax commented Nov 2, 2020

I'm on it.

@Redirion
Copy link
Member

Redirion commented Nov 2, 2020

its a bit more complicated this time to upgrade ExoPlayer. We need to provide a MediaItem. And media2 is incompatible with the MediaButtonReceiver. I did not find any replacement for that yet (or don't know if this is even required). For now we could just try ExoPlayer and investigate media2 afterwards, but we still need that MediaItem. I wanted to take a look at ExoPlayer's internals for that, as it converts MediaItems back to MediaSources for playback. Therefore it should hopefully not be hard to also get it working the other way around so that we can return suitable data there.
Are you okay with this approach or do you have other opinions?

@triallax
Copy link
Contributor

triallax commented Nov 2, 2020

I tried everything I could think of and found no issues.

@opusforlife2
Copy link
Collaborator

change compileSdkVersion and targetSdkVersion from 29 to 30.

Are we ready for this? Don't we need the SAF PR as a prerequisite for this?

@Stypox
Copy link
Member Author

Stypox commented Nov 2, 2020

Don't we need the SAF PR as a prerequisite for this?

Oh, do we? I have no idea, maybe it's better if I revert that part and let somebody else change it with more knowledge about what changes

@Stypox Stypox force-pushed the update-dependencies branch from 49f1026 to 2c4befb Compare November 2, 2020 17:15
@Stypox
Copy link
Member Author

Stypox commented Nov 2, 2020

@opusforlife2 I reverted the change to the target sdk version

@MD77MD
Copy link

MD77MD commented Nov 2, 2020

not sure if related but clicking on any of the highlighted buttons in any empty channel would crash newpipe. other then that everything is working as far as I can see.

@Stypox
Copy link
Member Author

Stypox commented Nov 2, 2020

@MD77MD that also happens in 0.20.2, so it's not caused by this. #4555 fixes it.

@MD77MD
Copy link

MD77MD commented Nov 2, 2020

@MD77MD that also happens in 0.20.2, so it's not caused by this. #4555 fixes it.

oh i see, i went crazy trying everything like you asked.. i would never realize this in normal use... why would anyone play an empty channel 🤣

@Isira-Seneviratne
Copy link
Member

Isira-Seneviratne commented Nov 2, 2020

Maybe RxJavaBridge could be used? That way an alpha version of Room wouldn't have to be used.

@Stypox
Copy link
Member Author

Stypox commented Nov 4, 2020

@Isira-Seneviratne I think we are good using an alpha version of Room, since there are no open issues about the new RxJava3 support and there are no breaking changes regarding what we are already doing. Therefore I think using another library to achieve RxJava3 support, just to ditch it when upgrading Room to a stable 2.3 version, is not convenient ;-)

@Stypox Stypox force-pushed the update-dependencies branch from 2c4befb to 18cb631 Compare November 4, 2020 17:07
@Stypox
Copy link
Member Author

Stypox commented Nov 4, 2020

I fixed some ktlint errors arisen after rebase

@mqus
Copy link
Contributor

mqus commented Nov 4, 2020

Hi! I tested the provided apk on Android Kitkat(api19) and found no bugs.

@Isira-Seneviratne
Copy link
Member

@Stypox desugar_jdk_libs has also been updated to 1.1.0.

@Stypox Stypox force-pushed the update-dependencies branch from 18cb631 to 987b6d4 Compare November 5, 2020 20:19
@Stypox
Copy link
Member Author

Stypox commented Nov 5, 2020

I rebased after Java-8-date-time changes. I also enabled ktlint's autoformatting by default and used version 1.1.0 of desugar_jdk_libs (thank you @Isira-Seneviratne). Here is another APK: app-debug.zip

@Stypox Stypox linked an issue Nov 9, 2020 that may be closed by this pull request
@Isira-Seneviratne
Copy link
Member

I rebased after Java-8-date-time changes. I also enabled ktlint's autoformatting by default and used version 1.1.0 of desugar_jdk_libs (thank you @Isira-Seneviratne). Here is another APK: app-debug.zip

It's been updated again to 1.1.1.

@Stypox
Copy link
Member Author

Stypox commented Nov 20, 2020

Rebased and updated desugar_jdk_libs to 1.1.1: app-debug.zip

@Stypox Stypox force-pushed the update-dependencies branch 2 times, most recently from 017a846 to 095ec30 Compare November 20, 2020 15:06
@Stypox
Copy link
Member Author

Stypox commented Nov 20, 2020

Again: app-debug.zip

@Stypox
Copy link
Member Author

Stypox commented Nov 20, 2020

@TobiGr I think I tested this well enough, can it be merged before creating any new conflict?

@TobiGr
Copy link
Contributor

TobiGr commented Nov 22, 2020

uh yeah. we should merge. Can you resolve the conflicts on more time, please?

@Stypox Stypox force-pushed the update-dependencies branch from 095ec30 to 79e0a77 Compare November 22, 2020 12:06
@Stypox
Copy link
Member Author

Stypox commented Nov 22, 2020

I rebased again @TobiGr : app-debug.zip

@Stypox
Copy link
Member Author

Stypox commented Nov 22, 2020

Update checkstyle (8.36.2 -> 8.37) and mockito-core (3.5.13 -> 3.6.0): app-debug.zip

@Stypox
Copy link
Member Author

Stypox commented Nov 22, 2020

I checked manually every library we are using and they were all up to date except those two (which I updated). Since those library are not used by the code but only for testing purposes, there is no difference.
Oh, it's rebase time again ;-)

@Stypox Stypox force-pushed the update-dependencies branch from 557e8ab to a44f307 Compare November 22, 2020 13:03
@Stypox
Copy link
Member Author

Stypox commented Nov 22, 2020

Ok, rebased again, hopefully the last one ;-)
app-debug.zip

@TobiGr TobiGr merged commit 2937606 into TeamNewPipe:dev Nov 22, 2020
@Stypox Stypox deleted the update-dependencies branch August 4, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App crashes when playing back any content
8 participants