Conversation
| composeActivityVersion = "1.7.0" | ||
| activityVersion = "1.7.0" | ||
| materialDesignVersion = "1.8.0" | ||
| kotlinVersion = "2.2.0" |
There was a problem hiding this comment.
if library will be compiled with kotlin 2.2.0 then users that use kotlin 2.0 or kotlin 1.9 will lost access to library.
i not see any reason to increase minimal kotlin version in this library. maybe you have some reasons?
There was a problem hiding this comment.
Are we sure about this? I don't think upgrading to Kotlin 2.2.0 prevents users with lower versions from using the library 🤔
One important thing to note is that upgrading Kotlin improves many aspects of KMP in general in terms of configuration and setup.
This is the main reason why I upgraded to this version.
Also, upgrading to a newer version of Kotlin simply implies that if users want to use the latest release, they should also upgrade. Projects using KMP today should most likely not be using Kotlin versions lower than 2.0 🤔
Concerning all the other comments of using the configured moko plugins, since upgrading to Kotlin 2.2.0 changes various KMP configurations, these plugins would have to be upgraded beforehand which is why I didn't do it. If you want, I could take some time and update them but I'd need to be sure that upgrading to Kotlin 2+ is possible for you
| id("org.jetbrains.kotlin.multiplatform") | ||
| id("dev.icerock.moko.gradle.publication") | ||
| id("dev.icerock.moko.gradle.detekt") | ||
| alias(libs.plugins.detekt) |
There was a problem hiding this comment.
in moko detekt plugin already configured all required rules and dependencies. why you not use it?
| iosX64() | ||
| iosSimulatorArm64() | ||
| applyDefaultHierarchyTemplate() | ||
|
|
||
| with(this.sourceSets) { | ||
| // creation | ||
| createMainTest("ios") | ||
| listOf( | ||
| iosX64(), | ||
| iosArm64(), | ||
| iosSimulatorArm64(), | ||
| ) |
There was a problem hiding this comment.
we should use convention plugin to simplify build configs. now we have duplication in this logic in each build config
| applyDefaultHierarchyTemplate() | ||
|
|
||
| androidTarget { publishLibraryVariants("release", "debug") } | ||
|
|
||
| listOf( | ||
| iosX64(), | ||
| iosArm64(), | ||
| iosSimulatorArm64(), | ||
| ) |
There was a problem hiding this comment.
this should be from convention plugin too
| applyDefaultHierarchyTemplate() | ||
|
|
||
| androidTarget { publishLibraryVariants("release", "debug") } | ||
|
|
||
| listOf( | ||
| iosX64(), | ||
| iosArm64(), | ||
| iosSimulatorArm64(), | ||
| ) |
There was a problem hiding this comment.
this should be from convention plugin too
| applyDefaultHierarchyTemplate() | ||
|
|
||
| androidTarget { publishLibraryVariants("release", "debug") } | ||
|
|
||
| listOf( | ||
| iosX64(), | ||
| iosArm64(), | ||
| iosSimulatorArm64(), | ||
| ) |
There was a problem hiding this comment.
this should be from convention plugin too
| sourceSets { | ||
| commonMain { | ||
| dependencies { | ||
| api(projects.permissions) | ||
| api(compose.runtime) | ||
| } | ||
| } | ||
|
|
||
| androidMain { | ||
| dependencies { | ||
| implementation(libs.activity) | ||
| implementation(libs.composeUi) | ||
| implementation(libs.lifecycleRuntime) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
what reason to change small version of dependency declaration to this one?
| applyDefaultHierarchyTemplate() | ||
|
|
||
| androidTarget { publishLibraryVariants("release", "debug") } | ||
|
|
||
| listOf( | ||
| iosX64(), | ||
| iosArm64(), | ||
| iosSimulatorArm64(), | ||
| ) |
There was a problem hiding this comment.
this should be from convention plugin too
|
|
||
| android { | ||
| namespace = "dev.icerock.moko.permissions.gallery" | ||
| compileSdk = 36 |
There was a problem hiding this comment.
this provided from moko gradle setup.
you should rollback moko convention plugins.
| linkerOpts.add("-dead_strip") | ||
| linkerOpts.add("-force_load_swift_libs") | ||
| freeCompilerArgs += listOf( | ||
| "-Xbinary=bundleId=dev.icerock.moko.sample.permissions", | ||
| ) |
There was a problem hiding this comment.
is it required? what this lines solve?
Updated all versions since this project is severly outdated