-
-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements #83
Improvements #83
Conversation
…the multiplatform plugin)
…Studio (or IntelliJ IDEA)
…ch as `latest`, `fastest`, `cutest`, `shortest`, etc.
@yshrsmz are you going to be able to review this, or should @jasonsparc split it up? |
I'm super busy these days, so can't say anything about ETA but wanting to review this. Sorry |
Sorry for being late.
We need to check the compatibility with the new Android source set layout introduced in Kotlin 1.8.0. https://kotlinlang.org/docs/multiplatform-android-layout.html#check-the-compatibility
Our primary focus is Kotlin Multiplatform, so I'm leaning towards downgrading AGP to 7.x, if AGP 8.x can handle plugins developed with AGP 7.x. KGP/AGP/Gradle compatibility table: https://kotlinlang.org/docs/gradle-configure-project.html#apply-the-plugin edit: yeah, we should downgrade AGP according to the table |
Major changes:
Android & JVM support – see, Support other Kotlin plugins #40
Made it easier to support more Kotlin plugins.
No longer filters out non-test compilations whose name ends with
*test
, such aslatest
,fastest
,cutest
,shortest
, etc.Minor changes:
Bumped Gradle to
8.0.2
(from8.0.1
)Some error & warning suppressions.
Some code reorganizations.
Remarks:
I may submit another PR after this, in order to add support for the Kotlin/JS plugin (which I believe, might be a small change compared to this PR).
I'll leave it up to you to give an appropriate version for the Kotlin Gradle Plugin (as I decided not to change it).
Right now, your plugin is targeting Kotlin
1.8.X
; however, I believe that lowering the version to support at least1.7.X
would've been better, as there are a lot of plugins out there that targets that version. But then again, this is just my recommendation; I'll leave this up to you to consider.Also, the Android Gradle Plugin is only used in tests and samples, and thus, I didn't bother touching it. It's okay for its version to be too high (so long as it's not too high that the build would warn you about the Kotlin Gradle Plugin's version being too low).
The
README
would need to be updated to reflect the new Kotlin version requirements.This PR might be a bit big. It may be easier to visualize the changes by using the IDE's diffing features (or at least use any other tool instead of relying on GitHub), as some changes are either indentation changes or moving files.
There are now 8 sample projects (4 KTS + 4 Groovy), instead of just 2. Maintaining them might be a hassle without IDE refactoring support. I'll leave it up to you on how to best organize them to have the IDE support integrated.
Perhaps you should integrate them with the main project, and if you do, be warned that you might have to change some
rootProject.name
properties in both of the 'sample' projects' settings file, as both are separate included builds.My recommendation is to just merge the two 'sample' included builds into a single included build. I don't know how best to do that as you might have your own preferences; thus, I'll leave this up to you.
All these are just my recommendation. Feel free to disregard them (or some).