Skip to content
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

Simplify Kotlin DSL setup #531

Merged
merged 10 commits into from
Nov 4, 2021
Merged

Simplify Kotlin DSL setup #531

merged 10 commits into from
Nov 4, 2021

Conversation

wolfs
Copy link
Contributor

@wolfs wolfs commented Oct 28, 2021

I got quite a few warnings when trying to upgrade Gradle to 6.9.1 on the build for #530. I was wondering if using the kotlin-dsl plugin could work, so less manual configuration is necessary.

It seems like the compatibility class KtDslCompatibilityUtils was only there for Kotlin DSL versions prior to Gradle 5.0, and the plugin only supports Gradle 5.6. So I wonder if the changes I did would work and maybe simplify things.

Latest 6.x version
Since Gradle sets the maximum heap to 64m now, the minimum heap can't
be set to 128M.
@wolfs wolfs changed the title Clean up kotlin dsl Simplify Kotlin DSL setup Oct 28, 2021
@ejona86
Copy link
Collaborator

ejona86 commented Oct 28, 2021

I see what seem to be good cleanups in here, but I don't see anything particular to what org.gradle.kotlin.kotlin-dsl is offering. I also didn't find much documentation for it.

@wolfs
Copy link
Contributor Author

wolfs commented Oct 28, 2021

I see what seem to be good cleanups in here, but I don't see anything particular to what org.gradle.kotlin.kotlin-dsl is offering. I also didn't find much documentation for it.

This is the documentation of the plugin and what it provides: https://docs.gradle.org/current/userguide/kotlin_dsl.html#sec:kotlin-dsl_plugin

Sadly, I don't think it is possible to omit the version number from Groovy DSL scripts :(. So I looked up the version which ships with Gradle 6.9.

@wolfs wolfs marked this pull request as ready for review October 29, 2021 12:41
Copy link
Collaborator

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks fair.

build.gradle Outdated Show resolved Hide resolved
.github/workflows/testing.sh Outdated Show resolved Hide resolved
build.gradle Show resolved Hide resolved
@wolfs
Copy link
Contributor Author

wolfs commented Nov 3, 2021

@ejona86 Thank you for the review, I addressed your comments. Please take another look!

- CI is set to true in Github action runs anyway.
- The memory settings shouldn't be necessary any more.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants