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
