[Adaptive] Migrate gradle module til Kotlin DSL #1579
Conversation
|
|
||
| lint { | ||
| textReport = true | ||
| textOutput = File("stdout") |
There was a problem hiding this comment.
I'm not quite sure about this, should this be set to the build folder maybe? 🤔
|
Thank you for these! Sorry I have been slow to review, I have been OOO this week. Just catching up but should be able to get to them soon. |
No worries at all! Take your time! :) |
adaptive/build.gradle.kts
Outdated
| } | ||
|
|
||
| compileOptions { | ||
| sourceCompatibility = JavaVersion.VERSION_1_8 |
There was a problem hiding this comment.
Let's make it JavaVersion.Version_11, let me know if there is something we stuck on Java 8
There was a problem hiding this comment.
Good idea! I think this went fine :) Upgraded it in 6bbdef1 :)
adaptive/build.gradle.kts
Outdated
|
|
||
| compileOptions { | ||
| sourceCompatibility = JavaVersion.VERSION_1_8 | ||
| targetCompatibility = JavaVersion.VERSION_1_8 |
adaptive/build.gradle.kts
Outdated
| testImplementation(libs.robolectric) | ||
| } | ||
|
|
||
| apply(plugin = "com.vanniktech.maven.publish") |
There was a problem hiding this comment.
Lets use the plugins block https://developer.android.com/build/migrate-to-kotlin-dsl#migrate-buildscript
adaptive/build.gradle.kts
Outdated
| @file:Suppress("UnstableApiUsage") | ||
|
|
||
| plugins { | ||
| id("com.android.library") |
There was a problem hiding this comment.
Let's add plugins to catalog as well - https://github.com/google/accompanist/blob/main/gradle/libs.versions.toml, here is how https://developer.android.com/build/migrate-to-catalogs#migrate-plugins
There was a problem hiding this comment.
I tried this, but since we are on agp version 7.4.1 I think we have this issue (gradle/gradle#17968) since we have to maintain the class path version as well. I can do a workaround like this
id(libs.plugins.android.library.get().pluginId) would that be okay?
There was a problem hiding this comment.
Yes please, as we continue to enjoy the catalogs, and we can change this code once the Gradle bug is fixed
There was a problem hiding this comment.
On the same thought we should move to AGP 8.0 which is stable
There was a problem hiding this comment.
Added this in 6bbdef1 :) Moving to AGP 8.0, should that maybe be in a separate PR if these are squashed in case it breaks something? Wdyt? :)
There was a problem hiding this comment.
I can take a look into upgrading to AGP 8.0 parallel to this :)
There was a problem hiding this comment.
Added an attempt on this here: #1590
There was a problem hiding this comment.
Does this now solve the need for using pluginId?
There was a problem hiding this comment.
I tried to rebase this locally and upgrading does not fix it. I think the fix is to migrate all the plugins at classpath to use the alias and then update this to use alias as well. The reason I think this is that the sync error is that it already has this plugin on the classpath with an unknown version, so the compatibility cannot be checked.
I can fix it if I add the pluginManagment block with repositories block to settings.gradle and then remove the classpath decleration and move the plugins to the plugins block using the alias there as well in the project gradle file. Should I do that here? Or wait until I migrate the project gradle and settings gradle file to Kotlin DSL? Or I can do it in a seperate PR if you want to seperate these two things.
There was a problem hiding this comment.
Let's wait until you migrate those other files and go with the current solution for now. I will merge this one in!
adaptive/build.gradle.kts
Outdated
| @@ -0,0 +1,134 @@ | |||
| /* | |||
| * Copyright 2022 The Android Open Source Project | |||
There was a problem hiding this comment.
Updated in 6bbdef1 :) Thanks!
|
This just needs rebasing I think. Can you please rebase and reopen a PR when ready? |
|
Ah yes, I'm not quite sure why this build before we bumped AGP 8.0.0, but I had to revert the Java target version in this module to 1.8 to get it to compile. I think this is because we are setting 1.8 in project gradle. Maybe it makes sense to migrate everything there to 11 in bulk? Sorry about this, I should have rebased it before and checked. |
This is the first PR in a series to migrate the gradle files to use Kotlin DSL instead of Groovy. Since having Kotlin DSL and Groovy in the same project works fine, I thought migrating module by module was okay.
The related issue is #1578.
I have split up the PR into two commits. The first one prepares the file to be migrated by using steps described in these docs. The second one is where I actually convert the file to
.ktsfile and updates the rest of the file to be compatible with Kotlin DSL.I have added a line to suppress unstable API usage, since a lot of the APIs here are marked incubating. What do you guys think about that?