-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Migrate project to Kotlin #289
Conversation
…on and more minor changes
|
@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:
Here are some things that scare me though:
|
|
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? |
|
@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 |
👆 I agree with all of this! Some thoughts:
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.) |
|
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 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 |
|
@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 @samtstern I consider that isn't a good practice to have tests in However I see as good to pursue |
|
Is there any progress in moving to Kotlin and publishing 4.X version? |
|
@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 |
|
@MadRatSRP that's true, but all of that can be accomplished via an |
|
@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 |
|
@vmadalin that sounds like a good plan. We can't add your repo under |
|
That sounds amazing @samtstern, I let you know when I'm done ;) |
|
@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! |
Description
Migrate all entire project to
Kotlin1.3.50, but keeping the integration concept. These are some of the things that have been done:ktlint,detekt,spotlessfor static analysis code adding them toTravisRationale/Settingsdialogs displaying anAlertDialogLowAPiPermissionsfor avoid throw exception to integrator on their codeOverview
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..