Migrate project to Kotlin

vmadalin commented 5 years ago

logo

Description

Migrate all entire project to Kotlin 1.3.50, but keeping the integration concept. These are some of the things that have been done:

Migrate entire sample to kotlin
Make the library more modular by structure better the files / code
Add a project logo, make more easy to identify it (It is just a proposal 😄 , please share you point of view)
Add severals tools like ktlint, detekt, spotless for static analysis code adding them to Travis
Migrate all the tests to kotlin, for guaranty the coverage for new code
Modify readme specify the minim API version, and increase the version to 3.1.0 (please review this part if you consider to make major change)
Simplify logic about Rationale/Settings dialogs displaying an AlertDialog
Eradicate LowAPiPermissions for avoid throw exception to integrator on their code

Overview

Architecture:
Coverage:

Next Steps

Obviously with these changes, we only migrate the project to kotlin with little changes or simplifications. It's a good point to continue in this line and try to simplify the integration of the library and not only also adding more coverage tests, code documentation, etc..

samtstern commented 5 years ago

@VMadalin wow! This is a massive and very well thought out contribution. First of all thank you for taking the time to do this. In general it's good to file an issue before sending a PR like this so you don't have to do any unnecessary work though.

Without looking too deeply here are some things I like:

The logo is awesome! Let's definitely use that.
The code reorganization is a good idea. Although we shouldn't move any classes which were part of the publicly documented API before, organization isn't worth breaking people's code.
I like moving the sample to Kotlin, shows off more modern practices.

Here are some things that scare me though:

Moving to Kotlin is definitely a breaking change and this would be 4.0.0. I am worried that doing this is mostly valuable to us and only causes churn for our developers. Is there anything about the API that gets much nicer in Kotlin? Should we consider just doing an easypermissions-ktx instead?
If we are going to do a breaking change, I would like to first fix as many of the open issues as we can in the Java version and release a 3.0.1 so that people can get a stable version of the library without adopting Kotlin.

samtstern commented 5 years ago

cc @SUPERCILEX who always knows what the Android community is up to. Would you say that it's "in fashion" for Java libraries to move to Kotlin or is that considered a hostile move?

vmadalin commented 5 years ago

@samtstern Thanks for your answer. I'm absolutely agree to focus effort trying to fix first the open issues, and upload a fix with these changes. Making more robust the current version. It's important also to migrate these fixes in the kotlin version.

Regarding the version, and repo. In my opinion it's not bad to think to create a new repository easypermissions-ktx or a new branch but the only problem what I see is to maintenance, both projects separately. Although while decide the best option maybe we should to create a specific branch called kotlin or similar and merge there all PR related and giving people the opportunity to contribute with next refactor version of the library

SUPERCILEX commented 5 years ago

The logo is awesome! Let's definitely use that.

The code reorganization is a good idea. Although we shouldn't move any classes which were part of the publicly documented API before, organization isn't worth breaking people's code.

I like moving the sample to Kotlin, shows off more modern practices.

👆 I agree with all of this!

Some thoughts:

Moving to Kotlin shouldn't be a breaking change. In theory, we can make the APIs exactly the same as before to keep supporting our Java users.
The main benefits of Kotlin are NPE avoidance, faster development cycles, and more pleasant/modern language to use that makes life easier.
- I would argue that we see none of those benefits. EasyPermissions is essentially a feature-complete library and thus in maintenance mode. That means we aren't actively developing it so a more enjoyable language that makes development faster is irrelevant. As for the NPEs we seen, they're mostly due to OEM bugs which means it would still be broken for Kotlin.
Kotlin presents a 1MB+ size increase for devs not using it previously. For something as fundamental as permissions, I feel like we'd want to minimize bloat and keep everybody happy (including Java people).
Major libraries that have switched to Kotlin are doing it because they can offer radically superior APIs (think Jetpack Compose, coroutines, operator + extension functions, property delegation, etc), but even that's sometimes unnecessary since you can usually just add extension functions in a companion lib to do the same thing (think KTX).
@samtstern There's also some confidential information I shared with you.

TL;DR: I would wholeheartedly support a KTX version of EasyPermissions, but I'm a little skeptical of having the whole library switched to Kotlin. Until 70% of the top 100K apps with updates in the last 6 months use Kotlin, I think libraries should generally stick to Java. (Or if JetPack releases a core library in Kotlin.)

samtstern commented 5 years ago

Thanks @SUPERCILEX, those are all great points and I agree.

@VMadalin I still think your work is very valuable so would it be possible to reduce this PR to just the changes to app code but not touch the core library yet? I am not comfortable with actually shipping any Kotlin code to our developers but I am happy to adopt Kotlin within the repository for our own productivity!

I would also be open to moving the tests to Kotlin (as you've already done) but I'm not sure if there is any way to do that and still safely ship a Java library so I think we have to hold on on that for now.

And then if you want to pursue easypermissions-ktx in the future let's open a new issue where we can discuss what the API would look like before we implement anything.

vmadalin commented 5 years ago

@SUPERCILEX I'm absolutely agree with all points that you comment. But obviously depends a lot of project factors. For example at Userzoom, we decided to migrated the SDK to Kotlin recently but the distribution of it is in .jar file, for keeping the compatibility with different frameworks/platforms like react-native, phonegap, xamarin and flutter. The tendencies in a long time is to migrate all projects but this is a big task and require time, and for the moment the tendency is to have a hybrid project (specially for big projects).

@samtstern I consider that isn't a good practice to have tests in kotlin and the library in java that create a lot of confusion on development process, specially if you use TDD or BDD. Maybe the only part what we could migrate is the sample, keeping or not the both samples. The project structure is also valuable base to start to discuss how it would be better to organise the project, facilitating its maintenance.

However I see as good to pursue easypermissions-ktx to start discuss the new API thinking in simplify the integration, for example it's a good start point compare another notification permission like: https://github.com/permissions-dispatcher/PermissionsDispatcher with a very interest integration method.

MadRatSRP commented 4 years ago

Is there any progress in moving to Kotlin and publishing 4.X version?

samtstern commented 4 years ago

@MadRatSRP no progress, I think the conversation above stands. I'd be open to converting the sample app and (maybe) the tests to Kotlin. I'd also be open to a KTX library. Converting the core library to Kotlin has no additional user benefit so I don't think it's a good investment.

MadRatSRP commented 4 years ago

@MadRatSRP no progress, I think the conversation above stands. I'd be open to converting the sample app and (maybe) the tests to Kotlin. I'd also be open to a KTX library. Converting the core library to Kotlin has no additional user benefit so I don't think it's a good investment.

Actually, it has a lof of the additional user benefit. As you know, current Android development flows around Kotlin (including Kotlin Native) and Flutter so if somebody will convert the whole library into Kotlin, that will make it much more popular. And your code will help someone to learn something new about Kotlin and other things

samtstern commented 4 years ago

@MadRatSRP that's true, but all of that can be accomplished via an easypermissions-ktx library, there's no need to convert the core library to Kotlin.

samtstern commented 3 years ago

@VMadalin hey it looks like you're back! Are you still planning to pursue this PR?

vmadalin commented 3 years ago

@VMadalin hey it looks like you're back! Are you still planning to pursue this PR?

Hi @samtstern, I was involved on other things this year and I decided to dedicate some time to move forward the work I did. My intention was to continue this under https://github.com/VMadalin/easypermissions-ktx, of course, keeping google licence.

I'm not sure if is possible to add under googlesample/easypermissions-ktx keeping the consistency with the java repo?

samtstern commented 3 years ago

@VMadalin that sounds like a good plan. We can't add your repo under googlesamples (that's for Google-owned code only) but if you want I am happy to link to https://github.com/VMadalin/easypermissions-ktx from the README of this repo.

vmadalin commented 3 years ago

That sounds amazing @samtstern, I let you know when I'm done ;)

samtstern commented 3 years ago

@VMadalin I'm going to close this PR since at this point we both agree we're not going to merge all of this. When you're ready please send a new PR to add a link to the README, or just send me an email (samstern at google dot com) and I'll do it!

googlesamples / easypermissions