-
Notifications
You must be signed in to change notification settings - Fork 40
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
Convert Gradle scripts from Groovy DSL to Kotlin DSL #232
Conversation
8b9f501
to
4d4cd70
Compare
Great! |
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.
Could you please rebase on a fresh master and resolve conflicts?
# Conflicts: # build.gradle.kts # settings.gradle
Done. Please could you review this soon? It's difficult trying to keep migrated buildscripts up to date and resolving conflicts, especially because this PR has renamed the files. |
} | ||
|
||
afterEvaluate { | ||
gradle.includedBuilds.forEach { included -> | ||
project(":kotlinx-benchmark-runtime").tasks.named("publishToMavenLocal") { dependsOn(included.task(":publishToMavenLocal")) } | ||
} | ||
} | ||
tasks.register("publishToMavenLocal") { |
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.
Does this make the afterEvaluate
block above redundant?
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.
I'm not sure, the build config is not easy to decipher.
The code in the afterEvaluate {}
block makes the :kotlinx-benchmark-runtime:publishToMavenLocal
task depend on the :publishToMavenLocal
task of every included build.
This means if any other task depends on :kotlinx-benchmark-runtime:publishToMavenLocal
, then it will also run the :publishToMavenLocal
task of every included build. So maybe if you run ./gradlew fooTask
and that requires :kotlinx-benchmark-runtime:publishToMavenLocal
, and that will trigger the other :publishToMavenLocal
tasks.
The tasks.register("publishToMavenLocal)
in the root build doesn't set up the same task dependencies, so they're not the same. It's more useful for being able to quickly run publishing to be able to check it works.
I want to come back and fix the build config, so it's easier to understand and work with. Converting the build scripts to use the Kotlin DSL is just one step of that.
The Kotlin DSL has many advantages over the Groovy DSL, making it easier to work with.
Some of kotlinx-benchmark's build scripts already use Kotlin DSL, and this PR converts the remaining ones.
I have intentionally tried to keep changes to a minimum, and focused on keeping the same behaviour. There should be no impact on the code or artifacts.
Part of https://youtrack.jetbrains.com/issue/KT-66764