After having written that I did a quick search, you can even use git blame with one big massive change commit [1], which would further help the idea of "just get over with it".
With that option, I'd even change all whitespaces if the community thinks that it's a better option (a separate discussion that I'll gladly skip). [1] https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame On Tue, Oct 6, 2020 at 5:38 PM Arvid Heise <ar...@ververica.com> wrote: > I'm also +1 for automatically enforceable code style. > > I also would just go over it as Chesnay said. While it makes some changes > a bit harder to track (inline git blame), it's easy to skip over in any git > history and if it's only one massive commit, then it's also much easier to > ignore than many gradual changes. Further, if we just do it once, git blame > will quickly become more reliable again. > > Btw I completely don't care about the code style as long as it plays well > with IntelliJ (it used to be different, but things change :p). > > On Tue, Oct 6, 2020 at 5:23 PM Chesnay Schepler <ches...@apache.org> > wrote: > >> We shouldn't switch to spaces _now_; cutting this bit from your proposal >> will massively simplify things and there's hardly any value in changing >> it. >> >> Also I'm getting rather tired of this constant idea of "gradual >> application". We've been doing this for 2-3 years now since we >> introduced Checkstyle and basically got nowhere. We should just bite the >> bullet and get it over with; we could've solved this whole problem >> already. >> >> In conclusion, I'm +1 on finally locking down the codestyle and applying >> it immediately, I'm -1 on any gradual application scheme because they >> _just don't work_. >> >> On 10/6/2020 2:15 PM, Aljoscha Krettek wrote: >> > Hi All, >> > >> > I know I know, but please keep reading because I recently learned >> > about some new developments in the area of coding-style automation. >> > >> > The tool I would propose we use is Spotless >> > (https://github.com/diffplug/spotless). This doesn't come with a >> > formatter but allows using other popular formatters such as >> > google-java-format. The nice thing about Spotless is that it serves as >> > a verifier for CI but can also apply the configured style >> > automatically. That is, for the programmer all she has to do is `mvn >> > spotless:apply` to fix any style violations. >> > >> > An interesting feature, which was (somewhat) recently added is >> > "ratchet" >> > ( >> https://github.com/diffplug/spotless/blob/main/plugin-maven/README.md#ratchet). >> >> > With this, you can set up Spotless to only apply it's rules to files >> > that were changed after a configured commit. This would allow a >> > gradual application of the new coding style instead of one big change. >> > >> > If we decide to use Spotless, we would of course also have to decide >> > on a coding style. For this I would propose google-java-format, which >> > the flink-statefun project uses. The main difference from our current >> > "style" is that this uses spaces instead of tabs for indentation. By >> > default it would be 2 spaces but it can be configured to use 4 spaces >> > which would make code look more or less like our current style. There >> > are no more configuration knobs, so using tabs is not an option. >> > >> > Finally, why should we do this? I think most engineers agree that >> > having a common enforced style is good to have so I only want to >> > highlight a few thoughts here about things we could improve: >> > >> > - No more nits about coding style in reviews, this makes it easier >> > for both the reviewer and developer >> > >> > - No manual fixing of Checkstyle errors because Spotless can do that >> > automatically >> > >> > - Because Flink is such a big project little islands of coding style >> > have formed between people that commonly work on components. It can be >> > a nuisance when you work on a different component and then reviewers >> > don't like your typical coding style. And you first have to get used >> > to the slight differences in style when reading code. >> > >> > There are also downsides I see in this: >> > >> > - We break the history, but both "git blame" and modern IntelliJ can >> > ignore whitespace when attributing changes. So for files that are >> > already "well" formatted not much would change. >> > >> > - In the short-term it will be harder to apply changes both to master >> > and one of the release-x branches because formatting will be >> > different. I think this is not too hard though because Spotless can >> > automatically apply the style. >> > >> > In summary, we would have some short-term pain with this but I think >> > it would be good in the long run. What are your thoughts? >> > >> > Best, >> > Aljoscha >> > >> >> > > -- > > Arvid Heise | Senior Java Developer > > <https://www.ververica.com/> > > Follow us @VervericaData > > -- > > Join Flink Forward <https://flink-forward.org/> - The Apache Flink > Conference > > Stream Processing | Event Driven | Real Time > > -- > > Ververica GmbH | Invalidenstrasse 115, 10115 Berlin, Germany > > -- > Ververica GmbH > Registered at Amtsgericht Charlottenburg: HRB 158244 B > Managing Directors: Timothy Alexander Steinert, Yip Park Tung Jason, Ji > (Toni) Cheng > -- Arvid Heise | Senior Java Developer <https://www.ververica.com/> Follow us @VervericaData -- Join Flink Forward <https://flink-forward.org/> - The Apache Flink Conference Stream Processing | Event Driven | Real Time -- Ververica GmbH | Invalidenstrasse 115, 10115 Berlin, Germany -- Ververica GmbH Registered at Amtsgericht Charlottenburg: HRB 158244 B Managing Directors: Timothy Alexander Steinert, Yip Park Tung Jason, Ji (Toni) Cheng
