-
Notifications
You must be signed in to change notification settings - Fork 168
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
bump kotlin to 1.5.31, bump ktlint to 0.43.2 #597
Conversation
set the minimum version of gradle to 6.8 previous versions of gradle don't support kotlin 1.4. Kotlin 1.4 is the minimum for ktlint 0.43.2 add changelog
@@ -10,22 +10,11 @@ import org.jlleitschuh.gradle.ktlint.testdsl.build | |||
import org.jlleitschuh.gradle.ktlint.testdsl.project | |||
import org.junit.jupiter.api.DisplayName | |||
|
|||
@GradleTestVersions(minVersion = "6.6.1") |
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 is no longer needed, because min gradle version is 6.8
* See https://docs.gradle.org/6.6-milestone-3/userguide/configuration_cache.html#config_cache:requirements:build_listeners | ||
* ``` | ||
*/ | ||
private val maxProblemsFlag = "-Dorg.gradle.unsafe.configuration-cache.max-problems=2" |
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 am not sure if it is still applicable, I run the tests with the value set to 0 and everything worked correctly.
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.
Do we want to restore and set the value to 0?
@JLLeitschuh Could you review the PR? 🙏 |
@JLLeitschuh up :) |
@@ -63,6 +63,6 @@ open class KtlintBasePlugin : Plugin<Project> { | |||
} | |||
|
|||
companion object { | |||
const val LOWEST_SUPPORTED_GRADLE_VERSION = "6.0" | |||
const val LOWEST_SUPPORTED_GRADLE_VERSION = "6.8" |
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 this consistent with the Android Gradle Plugin?
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.
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.
Just the one question, other than that, this looks good to me
@JLLeitschuh Can you once again run the tests against windows? |
It's not. Windows fail with an OOM error that I haven't figured out yet |
So, are we gonna merge this PR, even with failing windows build? ;) |
Thank you for your contribution. Sorry for taking so long. I've been super busy with Black Hat and DEFCON prepping as a speaker the past month. |
Based on this, I decided to create a PR that would bump ktlint to newer version. Instead of trying to update to the newest version in one PR, I find it more convenient to split the update into more smaller parts.
Ktlint 0.43.2 requires at least kotlin 1.4 API. That's why I needed to change the minimum required gradle version to 6.8.
In next PR I would try to update gradle from 7.1.1 -> 7.2 :)