`git blame` can set a start commit [1] so you can actually blame before the format commit. It's integrated with IDEA[2] and GitHub Web UI[3][4] also.
Best, tison. [1] https://stackoverflow.com/questions/5098256/how-can-i-view-prior-commits-with-git-blame [2] https://www.jetbrains.com/help/idea/viewing-changes-information.html#annotating-previous-versions [3] https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view [4] https://github.com/refined-github/refined-github/issues/2175 Yunze Xu <x...@apache.org> 于2023年6月30日周五 11:38写道： > One thing I’m concerned about is that the code reformatting could bring > very > wide changes and it’s very unfriendly to the `git blame` command. > > The check style plugin was not applied to all modules as well. There were > many > PRs to apply the check style plugin for different modules. Take my PR [1] > as > example, which applies the check style plugin for the pulsar-client > module. There > are over 2000 lines changes. It’s hard to review and it also brings a > breaking > change that was missed by the reviewers. You can see a public class > `Murmur3_32Hash` > was renamed with `Murmur3Hash32`. > > [1] https://github.com/apache/pulsar/pull/13940 > > Thanks, > Yunze > > On Fri, Jun 30, 2023 at 10:48 AM Joo Hyuk Kim <beansk...@gmail.com> wrote: > > > > > Spotless has an IDEA plugin, but it's for Gradle task only. > > > > I saw this one also 🥲. > > > > > [1] can be used > > > > Nice, IMO having at least one will suffice 👍🏻. > > Thanks, > > joohyuk > > > > On Fri, Jun 30, 2023 at 11:43 AM tison <wander4...@gmail.com> wrote: > > > > > > IDE > > > > > > If using the palantir style, [1] can be used. Spotless has an IDEA > plugin > > > but it's for Gradle task only. > > > > > > Best, > > > tison. > > > > > > [1] https://plugins.jetbrains.com/plugin/13180-palantir-java-format > > > > > > > > > Joo Hyuk Kim <beansk...@gmail.com> 于2023年6月30日周五 10:38写道： > > > > > > > Sounds great. > > > > > > > > Another thing is about IDE. > > > > Pulsar site sort of directs contributors to use IntelliJ as shown in > > > > https://pulsar.apache.org/contribute/setup-ide/. > > > > It would be nice if there was IDE support (auto-format, or > shortkey-base) > > > > and we can add it to the setup-ide guide also. > > > > Do you know any? Or I can do some research if you don't. > > > > > > > > Thanks, > > > > joohyuk > > > > > > > > On Fri, Jun 30, 2023 at 10:58 AM tison <wander4...@gmail.com> wrote: > > > > > > > > > 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 > > > > > > > > > > > > >
