Closed husseinala closed 7 months ago
Thank you soooo much for this PR, @husseinala !
I'm very excited to review it, test it and provide feedback! I'll only be able to take a look at this by the end of the week though, probably over the weekend!
I know there are many people interested in this, so I'll tag them here so they can follow the evolution of this new feature/PR as it happens and can also provide valuable feedback and maybe even help us test a bit in diverse projects/work environments 😊
@JARMourato @humblerookie @Shafichariri @houmie @mrea1 @yuukiw00w @AF-cgi @mrea1 @marceltex 🙇
Thanks a lot again @husseinala, and congrats on your first contribution here! ❤️
PS: Great PR description, thanks for the thorough explanation! 🤩
Thank you @rogerluan! Looking forward to the feedback from everyone 😊
Note
Partial review for now!
I left some minor comments so far, as I haven't reviewed the core logic of the PR. Some things I noticed (also piggy backing on some of the comments that were already left above by other reviewers):
- We should be updating the README as well to reflect all the changes (e.g. remove the "coming soon" section, adding proper usage documentation for the new kotlin features, perhaps add some more instructions on how to adequate your project for kotlin generation, etc).
- Kotlin unit tests are super valuable as they guarantee the actual encoding/decoding is working as expected, and that it doesn't regress in the future :) could you add those?
- Do the generated kotlin files pass a standard kotlin linter out-of-the-box? One with a "widely adopted" style guide 😇
Last but not least, could you also implement an e2e test to be run in CI via Rakefile, like we're doing with Swift? These references will be helpful:
Looking forward to continue reviewing this PR when I have some extra time! It's looking promising! 🙌
Thanks @JARMourato, @marceltex & @rogerluan for taking the time to go through the PR 🙏🏼 , will work on addressing all the raised issues and push all the necessary changes, including making sure all the generated code passes Kotlin's official codeing conventions
AI Summary deactivated by husseinala
#53 - Add support to Android using Kotlin code generation - By husseinala 2 weeks ago
#54 - Update Code Climate action version - By rogerluan 1 weeks ago
#261 - Bump @slack/web-api from 6.8.0 to 6.8.1 - By dependabot[bot] 10 months ago
arkana is an open repo and Watermelon will serve it for free. 🍉🫶
Hi @rogerluan, sorry for the delay, I've been a bit busy the last two weeks, but I finally got around to making the requested changes which address the following points:
should_generate_gradle_build_file
is set to true
.test_kotlin
task. This required the addition of a couple of support files which I included inside spec/fixtures/kotlin
to be able to run the Kotlin tests outside a normal Android project.I made some changes to the Kotlin erb templates to adjust the code style/formatting, more specifically the line breaks 😊 I think they look better this way. Here's a visual diff of the changes if you're curious:
Yaay 🎉 thank you for taking the time to review and clean up the PR @rogerluan. Really glad I was able to contribute to this project! ❤️
Resolves #1
Description
Where I work, we've been using Arkana on our iOS projects and we're big fans of the project, so would like to adopt it for our Android projects too, which is why we thought it would be helpful to kick off the process of adding Kotlin support.
This PR adds a
KotlinCodeGenerator
and modifies the execution entry point to determine which generator to run based on the specified--lang
argument flag.The following changes have also been made:
Arkana Arguments
--lang
flag used to specify the generator to use for e.g.kotlin
. Defaults toswift
.Arkana YAML config
kotlin_package_name
option which is used by the Kotlin generator to determine the package name of the generated module. Defaults tocom.arkanakeys
.kotlin_sources_path
option which is used by the Kotlin generator to determine the path for generated Kotlin classes. Defaults to src/main/kotlin.should_generate_gradle_build_file
option which is useful if yourresult_path
is a path of an existing module and you would like to avoid generating a newbuild.gradle
file.Generated module structure
Generated sources
{namespace}Environment.kt
Contains interface declaration for the environment-specific keys.
{namespace}.kt
Contains the Keys object: