https://kotlinlang.org
Join Slack
I am working on ktlint-gradle compatibility for 0.49, and I have some questions
# ktlint
w
I am working on ktlint-gradle compatibility for 0.49, and I have some questions
First, I am wondering about Baseline
it looks like it was moved to cli, which I don't see published to maven central
but I need that api to read the baseline file and use it to filter the linting errors found in our various gradle custom reporters
unless there is another way to do this now?
s
hm...that may be a mistake (it not being in maven central)
actually there's nothing to publish in there except for 
Baseline
. we may just need to move it back into ktlint-core
i'm not sure why it was moved into ktlint-cli
w
Ok that would be great
s
hrm. the dependencies are slightly tangled. i'll need to take a look at this tomorrow
w
Thanks. No rush of course. I appreciate you taking the time
s
of course. I think I may end up breaking into a separate ktlint-baseline module if I can't find a more logical place for it
perhaps i can merge and you can check with the SNAPSHOT to see if it gets what you need?
w
I had one question on the PR, but overall looks good. Id be happy to test against the snapshot and let you know
s
lol some bad copypasta
w
that's what i suspected :)
p
Life would be so much easier if the gradle plugin was part of ktlint
s
@wakingrufus thanks for your patience; I just merged the PR. let me know if the SNAPSHOT is working
@Paul Woitaschek it's not out of the question; it was mostly us trying to figure out the resourcing. for now we're collaborating much more closely to try to minimize disruption
w
Thanks, ill take a look at the snapshot.
p
That's great to hear. With whom do you try to collaborate?
w
which repo is the snapshot published to?
s
jonathan and john, who are the current primary maintainers of this plugin https://github.com/JLLeitschuh/ktlint-gradle
w
@Paul Woitaschek regarding combining the projects: in the gradle ecosystem, there is a convention of keeping plugin version decoupled from the tool version. for example, you can use the latest version of ktlint-gradle with any version of ktlint. This is to allow users to use the right plugin for gradle compatibility that they need, without forcing down new behavior from the external tool, or being stuck on an old version of the tool in some cases. merging the ktlint and ktlint-gradle projects would not really simplify this, as all the work we do in ktlint-gradle to decouple from a single compile target of ktlint would still need to done
s
I am hopeful the churn will lessen once we have a 1.0 release and more of a commitment to stop constantly breaking the API :)
p
Do you think there is really the need for that? This is a thing that doesn't ship as part of a library. I don't care about compatibility here, to me these just belong together.
w
need might be a strong word, but yes I think there is value in that, and it is how practically every other gradle plugin for external tooling works, so it would be unexpected if this one did not work that way
@Sha Sha Chu the snapshot is lookign good. Ill let you know if i have any other issues
p
Due to holidays, I have missed this thread. I would like to discuss the necessity of exposing the Baseline functionality by Ktlint. The baseline functionality is something that is entirely encapsulated in the Ktlint CLI. The KtlintRuleEngine has no knowledge about the Baseline concept. So, I am wondering why anybody else would need to know about the Baseline. From the perspective of Ktlint CLI, the baseline should not be tempered with by any other tools. So my counter questions are: 1. Do you invoke Ktlint CLI or do you invoke the KtLintRuleEngine directly? 2. Wat kind of reporters are you meaning with 
filter the linting errors found in our various gradle custom reporters
? When using Ktlint CLI, the linting errors which are stored in the 
baseline.xml
are already filtered out before calling the reporters. 3. When calling KtLintRuleEngine directly, why do you want to filter out results formerly stored in the baseline of Ktlint CLI? I would like to understand this as exposing the Baseline does not seem logical to me at this moment. Also exposing it as a separate artifact does feel a bit weird when I look at the result architecture diagram.
s
I would think the baseline should be something that is accessible to reporters, not the cli specifically. it just says whether or not a specific issue is already known. but obviously your understanding of the overall arch is much deeper than mine
p
Ktlint CLI collects all lint errors from the 
lint
and 
format
tasks of 
KtLintRuleEngine
. After filtering out all lint errors that exists in 
baseline.xml
the remaining lint errors are sent to the reporters.
s
i think i'm likely just misunderstanding, but how do API consumers use the baseline then? @wakingrufus FYI
p
That is exactly my question as well.
w
wea re calling KtLintRuleEngine directly
we use baseline to serialize out the baseline and then read it in when generating reports, so we can remove baselined errors form the lint output when generating the reports
all of these operations deal with files to maximize gradle caching
I am open to delegating to other APIs in ktlint if they are available for this though
p
So basically you’re replicating functionality which is part of Ktlint CLI. That is fine and understandable from viewpoint to maximize Gradle caching. But as you already replicate functionality regarding filtering out baseline errors before sending it to the reporters, I would say that replicating (e.g. copy paste the Baseline class) to your project would be best. There seems to be little reason that the 
baseline.xml
as used by Ktlint CLI should be identical to the baseline that is used by the Gradle plugin. Or, is it essential because users of the Gradle Plugin also use Ktlint CLI for the same project? From the viewpoint of segregation of concerns, I would advocate that both Ktlint CLI and the Gradle plugin use custom (independent) implementations of the Baseline.
w
I am trying to figure this out, im still rather new to the codebase... but it seems we are using Reporter/ReporterV2 with 
ktlint-reporter-baseline
to create the baseline. so we are delegating to that for serialization, so it would make sense to also delegate for deserialization, but for that we need the 
com.pinterest.ktlint.cli.api.loadBaseline
method right? doing this would make our baselines continue to be compatible with CLI. that is useful when, for example, devs are using the plugin locally, but the ci process might be invoking the cli by other means
I could copy-paste Baseline.kt to my project I suppose, but if the format ever changes, I would break
p
I am trying to figure this out, im still rather new to the codebase... but it seems we are using Reporter/ReporterV2 with 
ktlint-reporter-baseline
to create the baseline.
This is actually a very good remark and the solution to the problem. The Baseline class should move the 
ktlint-reporter-baseline
. As a result, both the producer and consumer of the 
baseline.xml
are in the same module. As both the plugin and 
ktlint-cli
already depend on the 
ktlint-reporter-baseline
it will work well for both of us. The API stays clean and logical.
PR is merged. I plan to release 
0.49.1
this weekend.
w
awesome thanks so much!\
p
Well, lets thanks everybody in participating in this thread. Only together we build great stuff.
w
p
Cool! But maybe you can wait a little before releasing. There are two issues of potential relevance. I will send more details tomorrow.
w
Ok thanks for the heads up. I still have a couple things to improve in my PR before we merge anyway
p
So yesterday, I have promised to send more details. A Spotless developer mentioned in issue #2042 that the introduction of the value classes 
RuleId
and 
RuleSetId
lead to Java interoperability problem in Spotless. This problem will be solved by replacing the value classes by data classes. Technically, this is a breaking change (
0.50.x
). I would guess, that the impact for 
ktlint-gradle
is a simple recompilation. But releasing in 
0.50.x
would mean an additional layer of incompatibility that you might want to avoid by skipping the 
0.49.x
. For a normal user, e.g. a non-API consumer, a 
0.50.x
does not make sense at it contains no functional improvements and only a few bugfixes. For Spotless the 
0.50.x
release is not pressing as they can work around the problem with reflection but want to get rid of that. From a ktlint perspective, I would prefer to add some functional changes and then release 
0.50.x
somewhere late in June. But, I can also imagine that from 
ktlint-gradle
perspective a 
0.49.2
release would be preferred although a patch release should not be used for a breaking change. What would your preference be from the 
ktlint-gradle
perspective? Then there is another noteworthy issue #1981 about a deprecation in the upcoming Kotlin 1.9 version which will cause some rules to break. To assist API Consumers in finding those problems before the actual release, I have created a fix that allows you to disable the registration of the offending extension point to find all problems and mitigate them ahead in time. This issue does not necessarily need to be released immediately, because the snapshot version can be used (after merge #2044) to analyse if detekt rules require to be changed.
w
Ok thanks for the update. From our perspective, there isnt a huge difference in 0.49.2 vs 0.50.0 since we look at the semantic version down to the patch version anyway (there are certain patch versions we don't support and tell people to upgrade to the fixed version. 0.49.0->0.49.1 will be an example of that.) We also are already sharing implementations across multiple minor versions. For example, the old rule provider api is used for 0.45.x, 0.46.x, 0.46.x, etc. So I'd lean slightly toward a 0.50.0 version since we dont yet have different impl across patch versions, just a "block list" of certain patch versions. But either is fine for us. Thanks for running it by us.
p
Ok, for now there is no pressing need from any party to release this fix a.s.a.p. So it will be deployed with the next release. Up until now there is no need for another patch release on the 
0.49.x
release.
37 Views
Open in Slack
PreviousNext
Powered by

kotlinlang

A modern programming language that makes developers happier.

Powered by