fossasia / open-event-droidgen

Open Event Android App Generator https://github.com/fossasia/open-event-android/raw/apk/sample-apk-fossasia17-development.apk
GNU General Public License v3.0
2.06k stars 828 forks source link

Remove Lombok Support from the project #2371

Closed sriramr98 closed 6 years ago

sriramr98 commented 6 years ago

Lombok is an unnecessary dependency that can be removed. Getters and setters can now be generated by Android Studio and to proceed with issue #2357 , we need to remove Lombok support. I propose we remove lombok and manually add Getters and Setters. Once kotlin is added, we will not need those anymore.

Would you like to work on the issue?

Yes

Target Milestone to solve the issue

I will be fixing this till GSOC'18

iamareebjamal commented 6 years ago

No, lombok is not just used for getters and setters. Android Studio was always capable of generating code. Generating getters and setters mean maintaining them as well. Kotlin may be added to project but it will only generate for data classes and nothing else. Also, builder, delegates and other stuff is not generated by kotlin

Secondly, why do we need to remove lombok for #2357 ?

Read this article for why kotlin data classes are not a replacement for lombok https://blog.fossasia.org/shrinking-model-classes-boilerplate-in-open-event-android-projects/

sriramr98 commented 6 years ago

Yeah but the thing is, if we add Kotlin to the project, the project will use Kotlin annotation processor by default and Lombok stops working. To add Kotlin support to the project, we will need to remove Lombok from the project. Maintaining getters and setters won't be a problem when we use data classes from kotlin.

iamareebjamal commented 6 years ago

I don't understand how lombok will stop working if kotlin is added. And btw, data classes getters and setters are not generated through annotation processors in kotlin.

The correct way of proceeding with a dependency removal is to firstly migrate all code from the project to the new format and then analyse if previous dependency is needed or not. As I stated in my previous comment, we are using lombok for other tasks than just generating getters and setters. And in #2357, I mentioned POJOs may very well be rewritten in kotlin, but it is already being done in a separate project. Duplicating efforts won't be fruitful

Once you have migrated the project to kotlin and if there is no need for lombok, then we can remove it. Making it a preleminary step for adding kotlin is a no go

sriramr98 commented 6 years ago

No, what I meant to say is that, when we add Kotlin support to the project, we will need to add kotlin-kapt plugin in the app module. This for some reason isn't recognizing the annotations from lombok and hence the getters and setters aren't being used. So when I tried to run the project, I got about 101 un found methods which correspond to these generated methods.

iamareebjamal commented 6 years ago

https://github.com/rzwitserloot/lombok/issues/1169

sriramr98 commented 6 years ago

Ah. This is better. Okay, I will try this and submit a PR with two Utility classes converted to Kotlin.