I find the ratchet feature you're suggesting interesting. But Arvid has a
point referring to the blog post about ignoring revisions in git blame [1].
Adding the configuration file for commits to ignore revs as proposed in the
blog post makes it even easier. One problem I see is that this is not
supported by Github (yet?) [2] as mentioned in [1].

Considering all that I prefer applying the code style in one go. I have no
strong opinion on what codestyle is the best.

PS: We used spotless in the project I previously worked on. It was
convenient to use.

[1]
https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
[2]
https://github.community/t/support-ignore-revs-file-in-githubs-blame-view/3256

On Tue, Oct 6, 2020 at 6:00 PM Aljoscha Krettek <aljos...@apache.org> wrote:

> Maybe I wasn't very clear on how the ratchet works. The changes are
> gradual yes, but it doesn't leave you an option: if you touch a file you
> will it will have to conform to the coding style afterwards. It's not
> like the previous gradual process we had before where it would be based
> on people actively working towards a style.
>
> That being said, I also completely like the option of just doing one big
> change commit.
>
> Regarding actual coding styles: we're a bit limited by what tools exist.
> I like Spotless because it can be used to both check and apply a style.
> Then you need a formatter that works with Spotless and of those we only
> have the Eclipse Formatter, google-java-format, and Prettier. Prettier
> is a Javascript tool that I would like to avoid. Eclipse is doable but
> you need to fiddle with configuration files. I like google-java-format
> because of it's take-it-or-leave-it approach. You either use the style
> or you don't but it's very thorough. The downside is that it won't do
> tabs-only formatting.
>
> Best,
> Aljoscha
>
> On 06.10.20 17:43, Arvid Heise wrote:
> > 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
> >>
> >
> >
>

Reply via email to