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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump ktlint to 0.45.2 #243

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Conversation

mateuszkwiecinski
Copy link
Collaborator

I might have misunderstood 0.45.0's release notes, but it seems like ktlint 0.45.1 ignores passed userData. To make the tests pass, I had to use the experimental methods and pass the indent_size via editorConfigOverride 馃憖

Assuming ktlint now has much better integration via .editorconfig then maybe it'd make sense to drop the custom indentSize extension within the plugin?

@mateuszkwiecinski
Copy link
Collaborator Author

Not sure if that's in mergeable state, but I went to ask around about these changes here: https://kotlinlang.slack.com/archives/CKS3XG0LS/p1647897839331439

@jeremymailen
Copy link
Owner

Thank you for all the research. I did peek at the thread in Kotlin slack. Has anyone provided guidance yet on the future of userData and lint method? The current API in ktlint tbh is rather squishy and if they are changing it, one hopes the change would make it a more solid API boundary with better documented expectations about what should happen.

Agree that we could drop custom params if editorconfig becomes the sole source of truth. I think that would warrant a 4.0 release if we do that.

@mateuszkwiecinski
Copy link
Collaborator Author

Has anyone provided guidance yet on the future of userData and lint method?

Unfortunately not :/

The current API in ktlint tbh is rather squishy and if they are changing it, one hopes the change would make it a more solid API boundary with better documented expectations about what should happen.

Yeah, that's what I would expect as well 馃憤
From what I understand, our options at this moment are:

  1. Drop the indentSize extension and keep using the deprecated lint and format methods. It's a breaking change as the plugin's public api changes, but at least plugin users could still override ktlint version
  2. Switch to ExperimentalParams like I did in this PR. Still a breaking change in cases when someone wants to downgrade ktlint version, plus we open ourselves to more unstable changes as the plugin would be relying on experimental features 馃し
  3. Wait until someone clarifies my doubts on the #ktlint channel (I will file a Github issue if I don't get any response there)

I can prepare a PR with the 1) approach somewhen later this week, so you could take a look how it would look like.
The decision is up to you of course. I'm only trying to be helpful ;)

@jeremymailen
Copy link
Owner

It might be perhaps worth waiting for pinterest/ktlint#1442 to resolve?

@mateuszkwiecinski
Copy link
Collaborator Author

Agree 馃憤
I'll revert all the changes here, and I'm going to propose to keep using the deprecated classes (mainly for backwards compatibility). Hopefully they'll address my concerns I raised in: pinterest/ktlint#1434 (comment) regarding deprecated/unstable classes 馃憖

@mateuszkwiecinski
Copy link
Collaborator Author

I reverted all the changes and only bumped ktlint to 0.45.2. The plugin will use deprecated classes, which likely will be removed in the next ktlint release - but I think it still makes sense to preserve backwards compatibility and switch to the new APIs, once they get more stable.
The build will now produce following warnings:

w: /home/runner/work/kotlinter-gradle/kotlinter-gradle/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/format/FormatWorkerAction.kt: (78, 23): 'format(KtLint.Params): String' is deprecated. Marked for removal in Ktlint 0.46. Move '.editorconfig' properties from 'Params.userData' to 'ExperimentalParam.editorConfigOverride'.
w: /home/runner/work/kotlinter-gradle/kotlinter-gradle/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/format/FormatWorkerAction.kt: (79, 20): 'Params' is deprecated. Marked for removal in Ktlint 0.46.
w: /home/runner/work/kotlinter-gradle/kotlinter-gradle/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/lint/LintWorkerAction.kt: (78, 16): 'lint(KtLint.Params): Unit' is deprecated. Marked for removal in Ktlint 0.46. Move '.editorconfig' properties from 'Params.userData' to 'ExperimentalParam.editorConfigOverride'.
w: /home/runner/work/kotlinter-gradle/kotlinter-gradle/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/lint/LintWorkerAction.kt: (79, 20): 'Params' is deprecated. Marked for removal in Ktlint 0.46.

@mateuszkwiecinski mateuszkwiecinski marked this pull request as ready for review April 6, 2022 16:45
@mateuszkwiecinski mateuszkwiecinski changed the title Bump ktlint to 0.45.1 Bump ktlint to 0.45.2 Apr 6, 2022
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 -- kicks the can down the road, but seems like that's the right thing for now.

@jeremymailen jeremymailen merged commit bc5279e into jeremymailen:master Apr 8, 2022
@mateuszkwiecinski mateuszkwiecinski deleted the ktlint_bump branch April 8, 2022 07:14
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.

None yet

2 participants