-
-
Notifications
You must be signed in to change notification settings - Fork 389
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
Migrate to ktlint 0.47.1 or newer #2914
Comments
Since it seems that the current Ktlint plugin isn't being updated we should migrate to kotlinter. Most complicated part is changing CI since that config currently is not in the repo (maybe we should fix that first? 🤔). I'd wait with all that until we have less open PRs, so until after Tusky 20 is out. |
The plugin has been updated https://github.com/JLLeitschuh/ktlint-gradle/releases/tag/v11.1.0 |
Good to know. Migrating plugins is probably still worth considering, given JLLeitschuh/ktlint-gradle#569. I tried the new version, by:
As expected, hundreds of lint warnings.
To make merging changes easier on authors with currently open PRs I suspect the best way to do it is two distinct commits on First commit makes the version and Second commit modifies the existing code to make it lint clean. Authors of open PRs then have to do a 4 step process:
I think this will minimise the number of conflicts that authors of open PRs will see when they do the merge, and keeps the reviewable changes in the PR back to just the code that needs to be reviewed, without any extraneous formatting changes. |
given, JLLeitschuh/ktlint-gradle#622 try setting ktlint version to 0.47.1 for now. |
The org.jlleitschuh.gradle.ktlint plugin (ktlint-gradle) is using an old version of ktlint (0.43.2, per https://github.com/JLLeitschuh/ktlint-gradle/blob/master/plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/KtlintExtension.kt#L38).
At the time of writing the most recent version of ktlint is 0.47.1.
The different ktlint versions have different rulesets, with newer versions recognising new issues.
This causes problems if you try and use other tools that incorporate ktlint. For example, editing Tusky code with the IntelliJ ktlint plugin (https://plugins.jetbrains.com/plugin/15057-ktlint-unofficial-) installed reports problems with the code that
gradlew ktlintCheck
does not find.ktlint-gradle does not work with any version of ktlint > 0.45.x at the moment (JLLeitschuh/ktlint-gradle#589).
Other ktlint + gradle options exist (supporting 0.47.1), including:
I experimented with the first one with kotlinter-gradle.patch.txt (probably needs some work, I did the first thing that works, not necessarily the best thing).
With that, running
gradlew lintKotlin
generates an additional ~ 280 lint warnings, see lint.txtThat probably doesn't make sense as a PR, as it touches the entire codebase and would need to be regenerated with every other PR that gets merged.
I think the next steps are;
The text was updated successfully, but these errors were encountered: