-
Notifications
You must be signed in to change notification settings - Fork 60
⬆️ Update versions #174
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
base: develop
Are you sure you want to change the base?
⬆️ Update versions #174
Conversation
composeActivityVersion = "1.7.0" | ||
activityVersion = "1.7.0" | ||
materialDesignVersion = "1.8.0" | ||
kotlinVersion = "2.2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be from convention plugin too
applyDefaultHierarchyTemplate() | ||
|
||
androidTarget { publishLibraryVariants("release", "debug") } | ||
|
||
listOf( | ||
iosX64(), | ||
iosArm64(), | ||
iosSimulatorArm64(), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be from convention plugin too
applyDefaultHierarchyTemplate() | ||
|
||
androidTarget { publishLibraryVariants("release", "debug") } | ||
|
||
listOf( | ||
iosX64(), | ||
iosArm64(), | ||
iosSimulatorArm64(), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be from convention plugin too
|
||
android { | ||
namespace = "dev.icerock.moko.permissions.gallery" | ||
compileSdk = 36 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it required? what this lines solve?
Updated all versions since this project is severly outdated