paug / AndroidMakersApp

📱Official companion app for Android Makers by droidcon 🇫🇷
https://androidmakers.droidcon.com/
117 stars 27 forks source link

Optimize and fix android dependencies #299

Closed cbeyls closed 1 month ago

cbeyls commented 1 month ago

This pull request optimizes and fixes the Android and Wear app dependencies and build configuration, with the following effects:

Detailed changes:

Please confirm if it's OK to remove the Firebase in-app messaging feature, otherwise AppCompat and Glide must be kept in the app just for it.

martinbonnin commented 1 month ago

Wow, nice! Thanks for this ❤️ ! I'll definitely need to take a deeper look but this looks great 👍 . TIL R8 could not tree shake the resources!

Initial comments:

Downgrade Apollo to 4.0.0-beta.2 because newer versions depend on version 2.0.0 of the Kotlin standard library but the project is designed to be built with Kotlin 1.9.23.

I think this is fine? kotlin-stdlib:2.0.0 is backward compatible with 1.9.23 so shouldn't be an issue to use a bigger version of kotlin. It could be an issue that the 1.9.23 compiler cannot read the 2.0.0 metadata but there's a n+1 best effort forward compatibility. So far it has not been an issue so I'm assuming it works for 1.9 -> 2.0 too?

Please confirm if it's OK to remove the Firebase in-app messaging feature

LGTM to remove them. We had plans to introduce some features but never got to it so I'm down for removing the dependency, We can alsways reintroduce later. Ping @enthuan? Ok for you?

cbeyls commented 1 month ago

I think this is fine? kotlin-stdlib:2.0.0 is backward compatible with 1.9.23 so shouldn't be an issue to use a bigger version of kotlin. It could be an issue that the 1.9.23 compiler cannot read the 2.0.0 metadata but there's a n+1 best effort forward compatibility. So far it has not been an issue so I'm assuming it works for 1.9 -> 2.0 too?

I didn't notice any issue, I just found suspicious that the entire app is using a beta version of the standard library that didn't match the compiler version, especially with the multiplatform stuff.

If you think it's safe then I can revert the change.

cbeyls commented 1 month ago

TIL R8 could not tree shake the resources!

I'm considering making a talk about limitations and workarounds of code and resources shrinking.

martinbonnin commented 1 month ago

especially with the multiplatform stuff

Right, native could be an issue (JVM has always proven to work quite well in the past). JS doesn't work for an example. If anything, I'll probably update this repo to use 2.0.0-RC1 and keep us moving.

I'm considering making a talk about limitations and workarounds of code and resources shrinking.

Looking forward to it :)

cbeyls commented 1 month ago

I focused on the Wear app and applied similar optimizations. I noticed a mismatch of Compose versions caused by Horologist, plus a bogus dependency from Horologist to both Material Components and AppCompat (!) and fixed these.

martinbonnin commented 1 month ago

AppCompat is everywhere!

martinbonnin commented 1 month ago

Follow up PR to bump Kotlin to 2.0.0-RC1: https://github.com/paug/AndroidMakersApp/pull/304

cbeyls commented 1 month ago

..., plus a bogus dependency from Horologist to both Material Components and AppCompat (!) and fixed these.

I created a pull request to fix the dependencies of Horologist 0.5.x: google/horologist#2215