https://kotlinlang.org logo
Join Slack
Powered by
Hey <@UL0CKFHUP>, Is there any way you can atte...
# ktlint
j
Hey @Sha Sha Chu, Is there any way you can attempt to limit the API breaking changes you're making? It's making it difficult to maintain Ktlint-Gradle in a way that doesn't require work on our behalf to support every version in some manual way. We've had to resort to quite a bit of reflection to maintain multi-version compatibility. CC: @wakingrufus
👍 1
s
hey @jlleitschuh @wakingrufus yeah let's chat about this? cc @Paul Dingemans
e
Maybe @nbadal is interested as well 😄
p
I know that the breaking API changes are painful. I am having the feeling that we are near the end in which Ktlint is in a good state for further development and breaking changes won’t occur every release. The next release (0.49) is the last one in which I expect a series of breaking changes. The aim of that release is preparing the module architecture of Ktlint in such a way that plugins can be integrated into the Ktlint project if that is still seen as something that is wanted. The next release after that could be the 1.0 version.
s
the other thing we can do is try to understand whether we can provide some kind of graceful fallback or other kinds of hooks in order to make these kinds of changes less painful
you're likely the biggest consumer of the API so it seems worth working together on the roadmap
w
👋 Hi there. Yeah it seemed like there was maybe a bit of thrashing from 0.45-0.48 about what the API should look like. if the new RuleSetProviderV2 api is going to be stable from now on, thats great. right now, a lot of the user data stuff was exposed via the ktlint-gradle gradle extension, so for the things like disabled_rules that have been removed, I think it makes sense to just steer users toward using .editorconfig directly, and then it doesnt need to be a concern of the plugin. once the API has been stable for awhile, we can "rebase" our support to drop support for the older versions, and consolidate on just using the new stable APIs without reflection. But I am interested in what the remaining breaking changes will be on the road to the 1.0 API
p
In a couple of days I will finish the PR containing the majority of the breaking changes that will be part of the 0.49 release. After that is merged there is still a lot to fix/implement for that release. It would be great if we can use that timeframe to check whether we need to make changes in that release to accommodate the API consumers.
w
sounds like a good plan. Let me know, and I can take a look at the changes
p
See diagram below for the refactored architecture of Ktlint 
0.49.x
. See https://github.com/pinterest/ktlint/blob/master/CHANGELOG.md for a description of the breaking changes. See https://pinterest.github.io/ktlint/install/snapshot-build/ for the snapshot build containing the changes. I am not planning any other breaking changes in this release, unless I encounter obvious mistakes. This would be really a good moment to verify the impact of the changes in 
0.49.x
as we can the release within certain limits if that is convenient. Let me know, what I can do to help you in this process. In https://github.com/pinterest/ktlint/milestone/21 you can monitor what other changes I am planning to implement before the next version will be released. I expect this to take between 3 to 8 weeks throughput time.
w
ok thanks! the stuff under 
0.48.2 - 2023-01-21
was all part of 0.48.0 already right?
p
It is all related to 
0.48.2
or older. I just note that due to an incorrect merge that the changelog is not correct. Everything above 
## [0.48.2] - Unreleased
should be taken into account. So in the the markdown file it is line 1 to 228 that describe the breaking changes. Sorry for the inconvenience. I will fix this tomorrow.
w
is there a way in the new API to pass a path to ktlint to an editorconfig file that is different than the default 
.editorconfig
? I'd like to avoid parsing the file myself in order pass it in as a EditorConfigDefaults if possible, as it seems like that is not what that param is meant to be used for
p
There is no need to parse the file yourself. I will need to lookup the details but afaik there is helper method on 
EditorConfigDefaults
that does this for you since a couple of releases.
w
yeah there is a companion method for it, but i would need to use reflection to access it since we support multiple ktlint versions, so i was hoping there was a way to just pass the path in
p
Prior to ktlint 
0.47.0
it was possible to specify the path to 
.editorconfig
in the 
ExperimentalParameters
. The usage of that parameter was however unclear and let to usages in which different API consumers had different expectations about that parameter. So it has been deprecated in 
0.47.0
and removed in 
0.48.0
. Re-introducing the parameter in the 
KtlintRuleEngine
would re-introduce the old problem so that does not seem the way forward. I don’t know how the plugin usually supports backward compatibility. Can the reflection problem be avoided by exposing some other functionality in a separate module to bridge the gap? This module could for example be maintained until Ktlint 1.0 is release which really seems to be a good to drop the backward compatibility guarantee of he plugin.
w
after 1.0, I do want to move up the baseline ktlint version support. yeah there might be ways to do that, but id rather do some reflection then figure out how to dynamically load different libraries, and have to build those libraries targeting various versions of ktlint. this solution should just be until we can rebase on the stable 1.0 api
thanks for the context though. i will consider this feature "removed upstream" and see if its feasible to remove from the plugin as well
p
@wakingrufus @mateusz.kwiecinski Would it be an idea to integrate one of the Gradle plugins into the Ktlint project instead of in separate projects? I would expect that backward compatibility problems you are now facing, will be gone by doing this. The plugin can be kept simpler when living in the same code base as KtLintRuleEngine. Actually, it would be an amazing feature to be released in KtLint 1.0. I am willing to think about postponing the release of 
0.49
and rebrand it to 
1.0
if we would agree on this. How would you feel about that?
w
I think that makes sense, overall (see detekt). However, for this particular issue, it might not have much effect. Most of our backwards compatibility issues in ktlint-gradle come from the fact that we maintain compatibility with older ktlint versions. I think that will be better post 1.0 anyway since we will bump up our minimum supported version to 1.0. And we are also evolving the plugin to do less and encourage users to use .editorconfig as much as possible which also will decrease the maintenance burden in the long run. I kind of think that if we were to integrate ktlint and ktlint-gradle, it would be best to do it post 1.0 after we have a chance to clean up all the old code for the old APIs. Or at least post 0.49, so we can do the same and make sure new usage is well documented and bugs fixed. Ultimately it would be up to @jlleitschuh as the owner of the repo.
p
Actually @jlleitschuh brought up the issue of integrating a while ago (sept 30th) himself in a different thread (https://kotlinlang.slack.com/archives/C044J67D4R1/p1664489368417669). Integrating now, would save you all the effort to make current plugin compatible with 
0.49
. Instead we would basically copy a stripped version of the current plugin into the ktlint project without any of the code that is needed to maintain backwards compatibility with older ktlint versions which would not make sense when they are combined in one project. The current plugin project can then become obsolete and frozen at 
0.48
.
w
Its a little more complex than that because we currently compile against old ktlint and use reflection to be compatible with the newer versions. Your method would be simple if it were the other way around, but as it is, we have some work to do first
p
Yes, I (now) do understand that the plugin needs to be reworked to use the latest ktlint for compilation. But I would expect that this change will be much easier when you don’t have to maintain backward compatibility. Basically you could ditch most of the reflection stuff you’re now doing to ensure that ktlint-gradle users can use old ktlint versions. But I have to admit that I totally don’t know what the plugin actually is doing.
j
Maintainers want to be able to update the plugin version independent of the Ktlint version
p
Would that still be so in case both ktlint and the plugin are released together?
w
Yeah
Its a standard thing for gradle plugins that call other tools
p
Okay. I am not a Gradle user (except in Ktlint project) so I am not aware of such conventions.
But doesn’t this also mean that the projects should be kept apart?
w
It can be achieved either way. The bigger concern i think would be target kotlin version and such. The plugin needs to align with the what the supported gradle versions embed
I think a separate project/repo actually does simplify some things. But combining the projects at an organizational/governance level might still be worth it in terms of bringing the community together, focusing efforts, easing collaboration, etc. Maybe under a "ktlint" umbrella organization? I think there will always be other things like spotless that also consume ktlint, and thats great, but it can also be nice to have an official reference implementation
e
Could also be benefits to having it in same repo. You could use composite builds to automate testing of the gradle plug-in with ktlint.
p
I don’t see why a combined governance structure would make things easier. In the end it are just volunteers like you and me that maintain different projects. Doing the same thing under a different governance structure won’t have much impact. Direct communication like we have now, is what make collaboration easier.
w
I agree thats the most important thing
Yeah there are things we can do to make the same repo work. It just might complicate some things a little bit.
Going back, i kind of think its the best to not drop old ktlint support until AFTER the new ktlint api is out and people have had time to update their code bases
p
Maybe we should take a different angle in this discussion. I think we all agree that the Ktlint ecosystem is of value to the community. We would like that our users can use Ktlint in a way that is most convenient for them. Either via Ktlint CLI, via a Gradle plugin, using Spotless or whatever.
The next question is what our biggest hurdle is. At this moment that would be the stability of the API which should be stable when releasing 
1.0
. Another hurdle is the availability of maintainers. I think it would be great if we can combine forces into a single repository with multiple active maintainers. The number of active maintainers in Ktlint is approximately 1 …
s
I'm glad to see this discussion happening. Paul has really singlehandedly been moving ktlint along, with very very minimal help from me. re combining into a single repo, I will likely defer to you all. I don't think it's necessarily required as long as there's close communication, but if it simplifies things I think we can likely fold them into a single repo.
w
Im still new to ktlint-gradle and it was fairly dormant for awhile, so im still trying to catch up on squashing bugs and 0.48 support. We also have gradle 8 support on our plate. After that, i do want to eventually drop support for older ktlints and change the complile target to 1.0, but only after that api is out and we have compatibility with that version of ktlint for awhile to give our users time to migrate. I dont think combining projects would change or simplify any of this. I don't want to take any shortcuts that would introduce friction for downstreams. Since ive started on the project, we are starting to get more outside contributions as well, so i want to foster that. After this is all done, its probably worth reconsidering.
p
Okay, I am fine with that. I will continue development and focus on 
1.0
version of the API. Lets keep in touch.
w
For sure!
p
I am closing in on releasing 
0.49.x
. Only one issue is left to implement and then regression testing starts. If you would like me to make adjustments to the API of the next release, you should do so asap. I expect to release the next version by the end of next week otherwise. The 
0.49.x
release can be seen as a release candidate for the 
1.0.x
version. The API for the 
1.0.x
should be identical to the 
0.49.x
with exception of removal of API’s which will be deprecated in 
0.49.x
.
6 Views
Open in Slack
PreviousNext
Powered by