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

Bump Ktlint to 0.47.1 (+ bump Kotlin to 1.7.10) #275

Merged
merged 4 commits into from
Sep 8, 2022

Conversation

mateuszkwiecinski
Copy link
Contributor

@mateuszkwiecinski mateuszkwiecinski commented Sep 6, 2022

The latest version of Ktlint deprecates some of the classes the plugin uses, so I migrated the code to the new apis.

This means Kotlinter will require Ktlint 0.47.0 and up 👀

@@ -11,7 +11,7 @@ internal fun editorConfigOverride(ktLintParams: KtLintParams): EditorConfigOverr
return if (rules.isEmpty()) {
EditorConfigOverride.emptyEditorConfigOverride
} else {
EditorConfigOverride.from(DefaultEditorConfigProperties.disabledRulesProperty to rules.joinToString(separator = ","))
EditorConfigOverride.from(DefaultEditorConfigProperties.ktlintDisabledRulesProperty to rules.joinToString(separator = ","))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +14 to +21
.sortedWith(
compareBy {
when (it.id) {
"standard" -> 0
else -> 1
}
},
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't understand why sorting was required, but I preserved the behavior, despite the new returned type is a Set

Copy link
Owner

Choose a reason for hiding this comment

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

At one point way back we intentionally put standard rules first in the list although I can't dig up the reason why. It might have been to report them first, but this might be an obsolete concern.

@mateuszkwiecinski mateuszkwiecinski marked this pull request as ready for review September 7, 2022 09:03
@jeremymailen jeremymailen self-requested a review September 8, 2022 04:55
Copy link
Owner

@jeremymailen jeremymailen left a comment

Choose a reason for hiding this comment

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

Thank you as always for the diligence!
Once we get this one in and maybe 1 or 2 other PRs hanging out, we can publish the next release.

  • Go to 0.47.1
  • Check compatibility with kotlin 1.7.0

@@ -2,7 +2,7 @@ import org.jetbrains.kotlin.gradle.plugin.getKotlinPluginVersion
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
kotlin("jvm") version "1.7.0"
kotlin("jvm") version "1.7.10"
Copy link
Owner

Choose a reason for hiding this comment

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

Out of curiosity, do we still work with 1.7.0 or do we need to update the compatibility matrix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I verified we're still compatible with kotlin 1.7.0

build.gradle.kts Outdated
@@ -27,7 +27,7 @@ description = projectDescription
object Versions {
const val androidTools = "7.2.1"
const val junit = "4.13.2"
const val ktlint = "0.46.1"
const val ktlint = "0.47.0"
Copy link
Owner

Choose a reason for hiding this comment

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

Let's go to 0.47.1!
I've gotten into the habit of waiting a few days after each ktlint release because there's always a flurry of error reports and fast fixes. Seems like some important ones to pick up.

Comment on lines +14 to +21
.sortedWith(
compareBy {
when (it.id) {
"standard" -> 0
else -> 1
}
},
)
Copy link
Owner

Choose a reason for hiding this comment

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

At one point way back we intentionally put standard rules first in the list although I can't dig up the reason why. It might have been to report them first, but this might be an obsolete concern.

@mateuszkwiecinski mateuszkwiecinski changed the title Bump Ktlint to 0.47.0 (+ bump Kotlin to 1.7.10) Bump Ktlint to 0.47.10 (+ bump Kotlin to 1.7.10) Sep 8, 2022
@mateuszkwiecinski mateuszkwiecinski changed the title Bump Ktlint to 0.47.10 (+ bump Kotlin to 1.7.10) Bump Ktlint to 0.47.1 (+ bump Kotlin to 1.7.10) Sep 8, 2022
@mateuszkwiecinski mateuszkwiecinski merged commit 21b86c3 into master Sep 8, 2022
@mateuszkwiecinski mateuszkwiecinski deleted the bump_ktlint branch September 8, 2022 07:25
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