Remove kotlin android extensions

malinajirka commented 3 years ago

This PR is built on top of https://github.com/Automattic/stories-android/pull/688. It removes no longer necessary "kotlin-android-extensions" which has been preventing us from updating to a newer gradle version.

This PR needs to be merged after https://github.com/Automattic/stories-android/pull/688 gets merged.

This is still a draft since I haven't tested the changes in WPAndroid.

Changes in this PR

Removes kotlin-android-extensions'
Updates kotlin from 1.4.1 to 1.4.2 so kotlin-parcelize plugin is available (release notes - AFAICT there are no changes which would affect us - We might need to update other repos to this kotlin version, but I assume we'll need to do that anyway)
Adds kotlin-parcelize plugin to keep support for @Parcelize annotation which used to be part of kotlin-android-extensions but now lives in a standalone plugin (kotlin-parcelize)
Suppresses "ParcelCreator" lint error - it's a false positive. Classes which use @Parcelize don't need to manually implement Creator.

To test: Smoke test the app

malinajirka commented 3 years ago

Merge of this PR is on hold until we migrate WPAndroid to kotlin 1.4.2. In order to do that we'll need to

Replace synthetic accessors with ViewBinding in WPAndroid (probably even its dependencies)
Remove all usages of apply plugin: 'kotlin-android-extensions'

ParaskP7 commented 3 years ago

👋 @malinajirka !

Merge of this PR is on hold until we migrate WPAndroid to kotlin 1.4.2.

Can you help me understand why is that? 🙏

malinajirka commented 3 years ago

Thanks so much for checking out this PR @ParaskP7.

Can you help me understand why is that? 🙏

'kotlin-android-extensions' and kotlin-parcelize can't live in the same project. Since stories are just a git submodule in WPAndroid, we need to remove all usages of 'kotlin-android-extensions' from WPAndroid before we'll be able to use Stories with kotlin-parcelize. At least that's how it looked when I tried to use this branch in WPAndroid - if I recall correctly the build actually worked, but AS was totally confused and some classes were underlined. Having said all that I didn't dive deeply into it as I knew we'd need to remove 'kotlin-android-extensions' in WPAndroid anyway.

ParaskP7 commented 3 years ago

👋 @malinajirka !

'kotlin-android-extensions' and kotlin-parcelize can't live in the same project. Since stories are just a git submodule in WPAndroid, we need to remove all usages of 'kotlin-android-extensions' from WPAndroid before we'll be able to use Stories with kotlin-parcelize.

🤔 I wouldn't think that to be the case, since in the end those are just story modules that will share their .aar with the downstream app. This is very interesting, thanks for the clarification. 🙏

At least that's how it looked when I tried to use this branch in WPAndroid - if I recall correctly the build actually worked, but AS was totally confused and some classes were underlined. Having said all that I didn't dive deeply into it as I knew we'd need to remove 'kotlin-android-extensions' in WPAndroid anyway.

🤔 This is very interesting, thanks for sharing. 🙏

As long as you have tested it and found this to be the case, I am fine to have this PR sitting as draft until we are done with the rest of kotlin-android-extensions in WPAndroid.

PS: @oguzkocer do we have any plans to also make the stories-android a composite build in the future? I kind of found it difficult to test this locally with WPAndroid as I didn't want to mess with submodules.

oguzkocer commented 3 years ago

I wouldn't think that to be the case, since in the end those are just story modules that will share their .aar with the downstream app. This is very interesting, thanks for the clarification.

@ParaskP7 I don't want to speak about this particular case without testing, but in our current setup, these modules are not any different from the libs/abc modules we have in WPAndroid. They are not standalone builds that are shared as an aar binary and does get affected by the configuration of the root project. Actually, if I am not mistaken, you can't even build some of the modules in isolation because they are not setup correctly for it.

PS: @oguzkocer do we have any plans to also make the stories-android a composite build in the future? I kind of found it difficult to test this locally with WPAndroid as I didn't want to mess with submodules.

Yes, it'll be gone after Gradle 7 upgrade work is done. Having said that, it isn't any harder to use submodules than to use composite builds. You basically check out the branch in libs/stories-android folder of WPAndroid. The only issue is that sometimes git will hate you for using submodules and will tell you something doesn't exist even though it does. So... yeah, it'll be gone soon :)

P.S: I love hearing about composite build being a success. I've had my worries 😟

ParaskP7 commented 3 years ago

👋 @oguzkocer !

@ParaskP7 I don't want to speak about this particular case without testing, but in our current setup, these modules are not any different from the libs/abc modules we have in WPAndroid. They are not standalone builds that are shared as an aar binary and does get affected by the configuration of the root project. Actually, if I am not mistaken, you can't even build some of the modules in isolation because they are not setup correctly for it.

Thanks for the clarification. 🙏

Yes, it'll be gone after Gradle 7 upgrade work is done. Having said that, it isn't any harder to use submodules than to use composite builds. You basically check out the branch in libs/stories-android folder of WPAndroid. The only issue is that sometimes git will hate you for using submodules and will tell you something doesn't exist even though it does. So... yeah, it'll be gone soon :)

git will hate you for... 😆 Thanks Oguz, yeap you are right, you can do that, thanks for sharing. It just scares me, I am not being able to trust git submodules lately, especially since I am not working regularly with them.

P.S: I love hearing about composite build being a success. I've had my worries 😟

Yeap, big success and you are a 🦸 ! MANY THANK 🙇

ParaskP7 commented 3 years ago

👋 @malinajirka @oguzkocer !

This PR can be now effectively unblocked as all the work within the parent Replace Synthetic Accessors with ViewBinding #14845 issue for WordPress-Android is done.

🍾

PS: Let me know if I can help with anything here.

malinajirka commented 3 years ago

I've rested this PR with the current develop of WPAndroid and I didn't encounter any issues - both the build and AS seemed to be happy with the current setup. I merging this PR, I'll create a PR in WPAndroid in a bit.

Automattic / stories-android

Remove kotlin android extensions #702