-
Notifications
You must be signed in to change notification settings - Fork 163
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
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 :)