My mistake. It looks that for some reason Spotless supports .editorconfig only 
for ktlint.

Best,
Kiryl

From: Kiryl Valkovich <kiryl_valkovich@teal.tools>
Date: Friday, June 30, 2023 at 6:45 AM
To: dev@pulsar.apache.org <dev@pulsar.apache.org>
Subject: Re: [DISCUSS] Consistent code style (esp. ws/indent) and autotools
Hi,

tison, are you going to use .editorconfig for specifying indent rules?

https://editorconfig.org/

It looks like Spotless supports it. 
https://github.com/diffplug/spotless/issues/734

Flink and many other ASF projects use it. 
https://github.com/apache/flink/blob/21eba4ca4cb235a2189c94cdbf3abcec5cde1e6e/.editorconfig
https://github.com/search?q=org%3Aapache%20.editorconfig&type=code

Unlike Spotless, the .editorconfig works out of the box in most of the modern 
code editors.

Best,
Kiryl

From: tison <wander4...@gmail.com>
Date: Friday, June 30, 2023 at 3:58 AM
To: Dev <dev@pulsar.apache.org>
Subject: [DISCUSS] Consistent code style (esp. ws/indent) and autotools
Hi,

I'd like to propose applying a consistent code style (especially whitespace
and indent) with an autotool Spotless.

// Background

Over and over we argue contributors reformat their patch manually for
checkstyle violations, or even whitespace changes that are not detected by
checkstyle. [1]

A common reason is that such style-only changes increase the burden to do
cherry-pick if a later bug fix is made around the code while we often do
not pick the style change barely or even it's coupled with an unpickable
commit.

CheckStyle helps in some cases while we don't have a strong rule set nor
even apply it to tests (porting bug fix actually include the test).

Manually fixing style violations can be boring and even annoying. That's
why it's effectively "suppressed" in mind.

An autotool to do the formatting work can help and here comes Spotless[2].
Flink and Curator use it and Flink actually migrated from CheckStyle to
Spotless for its more strict consistent rule set and oneliner format. See
their original discussion for the context[3].

// Proprosal

Using Spotless as the auto-formatting tool in all around apache/pulsar.
Since it's an auto tool I can cover the task solely.

// Concerns

1. Won't it be yet another noise to the codebase?

Yes and no. I suggest we pick this change to all maintained versions so
that all of them align with the consistent style. Then from now on, no more
style conflict.

Flink used the same strategy[4] and even we can do it starting from 2.10 as
Lari proposed to apply commits from the least recent maintained version. I
understand the maintained versions are: 2.10, 2.11, 3.0, and master.

2. Can it cover all the rule sets we use in CheckStyle now?

Let's say we almost don't care what the style is but it's consistent and
"not awful".

The default Google style takes line length = 80 which can be a
disappointment so in Curator I use the palantir style[5] which takes line
length = 120 which should keep consistent with current settings.

A downside is that Spotless Maven plugin cannot detect "forbidden imports"
where we can still use CheckStyle[6] - this is a low-traffic path and it
should be fine.

// Implementation

1. Introduce Spotless in the project and apply it around the codebase, for
all maintained versions.
2. Remove the then redundant CheckStyle rules, while retaining the
forbidden imports part as it's still useful.

Looking forward to your feedback!

Best,
tison.

[1]
https://github.com/apache/pulsar/pull/20642/files#diff-cb9b09b67f54fccdd5155dbbcedd50970ee93d9ee85ade1e6b6984cab64dab5d
[2] https://github.com/diffplug/spotless
[3] https://lists.apache.org/thread/3kjkcz9gj6f8j477d1t3gnbkl61hsb7z
[4] https://lists.apache.org/thread/nxdm0pg84q913w0kxszm502myqcg3db0
[5] https://github.com/palantir/palantir-java-format
[6] https://issues.apache.org/jira/browse/FLINK-32154

Reply via email to