[DISCUSS] Enforce common opinionated coding style using Spotless

[Yangze Guo]

+1
Thanks for driving this!

Best,
Yangze Guo

On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman <[hidden email]> wrote:

>
> +1 it works really well in StateFun for quite some time.
>
> On Thursday, December 17, 2020, Wei Zhong <[hidden email]> wrote:
>
> > +1 for the coding style automation. Thanks for driving this!
> >
> > Best,
> > Wei
> >
> > > 在 2020年12月17日，10:10，Xingbo Huang <[hidden email]> 写道：
> > >
> > > +1 asap. Spotless can greatly help us save the time of fixing checkstyle
> > > errors.
> > >
> > > Best,
> > > Xingbo
> > >
> > > Kostas Kloudas <[hidden email]> 于2020年12月17日周四 上午4:14写道：
> > >
> > >> +1 asap from my side as well.
> > >>
> > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <[hidden email]>
> > wrote:
> > >>>
> > >>> +1 asap.
> > >>>
> > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <[hidden email]>
> > >> wrote:
> > >>>
> > >>>> +1
> > >>>>
> > >>>> Thanks for driving this.
> > >>>>
> > >>>> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <[hidden email]>
> > >>>> wrote:
> > >>>>
> > >>>>> +1 to set this up ASAP. Now's a good time to (finally) finalize this
> > >>>>> effort with a new release cycle having started and
> > >> christmas/vacations
> > >>>>> being around the corner.
> > >>>>>
> > >>>>> On 12/16/2020 7:20 PM, Aljoscha Krettek wrote:
> > >>>>>> Let's try and conclude this discussion! I've prepared a PoC that
> > >> uses
> > >>>>>> Spotless with google-java-format to do the formatting:
> > >>>>>> https://github.com/aljoscha/flink/commits/flink-xxx-spotless
> > >>>>>>
> > >>>>>> To summarize from earlier discussion, the main benefits are:
> > >>>>>> - no more worrying about code style, both as reviewer and
> > >> implementer
> > >>>>>> - everyone can configure their IDE to auto-format, there will
> > >> never
> > >>>>>> be unrelated formatting changes
> > >>>>>>
> > >>>>>> Also, this uses git's blame.ignoreRevsFile to add a file that tells
> > >>>>>> git blame/IntelliJ to ignore the refactoring commit. However, you
> > >> need
> > >>>>>> to manually configure your git for that using:
> > >>>>>>
> > >>>>>> $ git config blame.ignoreRevsFile .git-blame-ignore-revs
> > >>>>>>
> > >>>>>> You can check out the PoC, look at the code in an IDE, play around
> > >>>>>> with blame/annotations.
> > >>>>>>
> > >>>>>> By the way, the coding style is not configurable, it’s the Google
> > >> Java
> > >>>>>> Style, wich uses spaces for indentation. In an IDE or on github the
> > >>>>>> code looks barely different from the previous formatting at a first
> > >>>>>> glance.
> > >>>>>>
> > >>>>>> For IDE setup, I will add a guide in the README but it boils down
> > >> to:
> > >>>>>>
> > >>>>>> - install the google-java-format plugin, enable it
> > >>>>>> - install the Save Actions plugin, enable "Optimize Imports" and
> > >>>>>> "Reformat File"
> > >>>>>>
> > >>>>>> With this, every time you save, formatting will be applied
> > >>>>>> automatically. You will never see formatting complaints from CI
> > >>>>>> (except for cases where you break semantical checkstyle rules,
> > >> such as
> > >>>>>> using certain imports that we don't allow.).
> > >>>>>>
> > >>>>>> What do you think?
> > >>>>>>
> > >>>>>> Best,
> > >>>>>> Aljoscha
> > >>>>>>
> > >>>>>> On 19.10.20 12:36, Aljoscha Krettek wrote:
> > >>>>>>> I don't like checkstyle because it cannot be easily applied from
> > >> the
> > >>>>>>> commandline. I'm happy to learn otherwise, though. And I'd also be
> > >>>>>>> very happy about alternative suggestions that can do that.
> > >>>>>>>
> > >>>>>>> Right now, I think Spotless is the most straightforward tool for
> > >>>>>>> that. Also I don't care so much about what style we choose in the
> > >>>>>>> end. (If we choose one). My main motivation is that we have a
> > >> common,
> > >>>>>>> strict style that can easily applied via tooling so that we no
> > >> longer
> > >>>>>>> need to comment on coding style in PRs.
> > >>>>>>>
> > >>>>>>> Aljoscha
> > >>>>>>>
> > >>>>>>> On 09.10.20 11:11, tison wrote:
> > >>>>>>>> +1 on locking on one codestyle and I think that is what current
> > >>>>>>>> checkstyle
> > >>>>>>>> rules serving.
> > >>>>>>>>
> > >>>>>>>> For automatic applying part, we suggest developing by IDEA and
> > >> with
> > >>>>>>>> Checkstyle Plugin on IDEA applying checkstyle suggestion is also
> > >>>>>>>> automatic.
> > >>>>>>>>
> > >>>>>>>> One short coming for spotless is that we can hardly adjust rules
> > >> if
> > >>>> the
> > >>>>>>>> project has its own issues to overcome. IIRC only several
> > >>>>>>>> pre-defined rules
> > >>>>>>>> and a clumsy general regex substitution rule can be used.
> > >>>>>>>>
> > >>>>>>>> FYI my team started with spotless but ended up with checkstyle
> > >> with
> > >>>> few
> > >>>>>>>> rules and Checkstyle Plugin for automation.
> > >>>>>>>>
> > >>>>>>>> Codestyle, in another perspective, is part of cognition of
> > >> developers
> > >>>>>>>> working in project, not something we just apply before pull
> > >> request.
> > >>>> No
> > >>>>>>>> matter how much automation introduced, most of developers will
> > >>>> converge
> > >>>>>>>> working with the configured codestyle.
> > >>>>>>>>
> > >>>>>>>> Best,
> > >>>>>>>> tison.
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> Kostas Kloudas <[hidden email]> 于2020年10月7日周三 下午6:37写道：
> > >>>>>>>>
> > >>>>>>>>> Hi all,
> > >>>>>>>>>
> > >>>>>>>>> +1 for enforcing "a" codestyle using "a" tool.
> > >>>>>>>>>
> > >>>>>>>>> As the project grows both in terms of LOCs and contributors,
> > >> this
> > >>>>>>>>> becomes more and more important as it eliminates some potential
> > >>>> points
> > >>>>>>>>> of friction without any additional effort.
> > >>>>>>>>>
> > >>>>>>>>> From the discussion, I am leaning towards having a single
> > >> commit
> > >>>> with
> > >>>>>>>>> all the codestyle-related changes. This will avoid sprinkling
> > >>>>>>>>> refactoring commits all over the place for the next year or
> > >> more.
> > >>>> But
> > >>>>>>>>> if this is the price to pay for having consensus on a tool,
> > >> then I
> > >>>> am
> > >>>>>>>>> ok with gradual implementation. I believe that the value added
> > >> by
> > >>>>>>>>> having an automated process of enforcing a codestyle exceeds the
> > >>>> cost
> > >>>>>>>>> of the nuisance of gradual refactoring.
> > >>>>>>>>>
> > >>>>>>>>> As for the actual format, I like the google-java-format but
> > >> again,
> > >>>> if
> > >>>>>>>>> the community agrees on a different one I would not oppose that
> > >> (as
> > >>>>>>>>> long as it does not use the same amount of indentation for
> > >> method
> > >>>> args
> > >>>>>>>>> and method body :P).
> > >>>>>>>>>
> > >>>>>>>>> Cheers,
> > >>>>>>>>> Kostas
> > >>>>>>>>>
> > >>>>>>>>> On Wed, Oct 7, 2020 at 10:26 AM Chesnay Schepler <
> > >>>> [hidden email]>
> > >>>>>>>>> wrote:
> > >>>>>>>>>>
> > >>>>>>>>>> To me, ratchet seems to combine the worst aspects of both
> > >>>> approaches.
> > >>>>>>>>>> You get disruptive changes, but only in singular files,
> > >>>>>>>>>> for something mundane as a typo fix or import change, which
> > >> would
> > >>>> be
> > >>>>>>>>>> annoying to keep separate from the actual functional changes
> > >> in a
> > >>>> PR.
> > >>>>>>>>>>
> > >>>>>>>>>> On 10/7/2020 10:04 AM, Matthias Pohl wrote:
> > >>>>>>>>>>> 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
> > >>>>>>>>>>> <[hidden email]>
> > >>>>>>>>> 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 <
> > >> [hidden email]
> > >>>>>
> > >>>>>>>>> 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
> > >>>>>>>>>>>>>> <[hidden email]
> > >>>>>>>>>>
> > >>>>>>>>>>>>>> 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
> > >>
> >
> >

[Zhu Zhu]

+1. It can be very helpful for future development. Thanks for driving this!

Thanks,
Zhu

Yangze Guo <[hidden email]> 于2020年12月17日周四 下午2:48写道：

> +1
> Thanks for driving this!
>
> Best,
> Yangze Guo
>
> On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman <[hidden email]> wrote:
> >
> > +1 it works really well in StateFun for quite some time.
> >
> > On Thursday, December 17, 2020, Wei Zhong <[hidden email]>
> wrote:
> >
> > > +1 for the coding style automation. Thanks for driving this!
> > >
> > > Best,
> > > Wei
> > >
> > > > 在 2020年12月17日，10:10，Xingbo Huang <[hidden email]> 写道：
> > > >
> > > > +1 asap. Spotless can greatly help us save the time of fixing
> checkstyle
> > > > errors.
> > > >
> > > > Best,
> > > > Xingbo
> > > >
> > > > Kostas Kloudas <[hidden email]> 于2020年12月17日周四 上午4:14写道：
> > > >
> > > >> +1 asap from my side as well.
> > > >>
> > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <[hidden email]>
> > > wrote:
> > > >>>
> > > >>> +1 asap.
> > > >>>
> > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> [hidden email]>
> > > >> wrote:
> > > >>>
> > > >>>> +1
> > > >>>>
> > > >>>> Thanks for driving this.
> > > >>>>
> > > >>>> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> [hidden email]>
> > > >>>> wrote:
> > > >>>>
> > > >>>>> +1 to set this up ASAP. Now's a good time to (finally) finalize
> this
> > > >>>>> effort with a new release cycle having started and
> > > >> christmas/vacations
> > > >>>>> being around the corner.
> > > >>>>>
> > > >>>>> On 12/16/2020 7:20 PM, Aljoscha Krettek wrote:
> > > >>>>>> Let's try and conclude this discussion! I've prepared a PoC that
> > > >> uses
> > > >>>>>> Spotless with google-java-format to do the formatting:
> > > >>>>>> https://github.com/aljoscha/flink/commits/flink-xxx-spotless
> > > >>>>>>
> > > >>>>>> To summarize from earlier discussion, the main benefits are:
> > > >>>>>> - no more worrying about code style, both as reviewer and
> > > >> implementer
> > > >>>>>> - everyone can configure their IDE to auto-format, there will
> > > >> never
> > > >>>>>> be unrelated formatting changes
> > > >>>>>>
> > > >>>>>> Also, this uses git's blame.ignoreRevsFile to add a file that
> tells
> > > >>>>>> git blame/IntelliJ to ignore the refactoring commit. However,
> you
> > > >> need
> > > >>>>>> to manually configure your git for that using:
> > > >>>>>>
> > > >>>>>> $ git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > >>>>>>
> > > >>>>>> You can check out the PoC, look at the code in an IDE, play
> around
> > > >>>>>> with blame/annotations.
> > > >>>>>>
> > > >>>>>> By the way, the coding style is not configurable, it’s the
> Google
> > > >> Java
> > > >>>>>> Style, wich uses spaces for indentation. In an IDE or on github
> the
> > > >>>>>> code looks barely different from the previous formatting at a
> first
> > > >>>>>> glance.
> > > >>>>>>
> > > >>>>>> For IDE setup, I will add a guide in the README but it boils
> down
> > > >> to:
> > > >>>>>>
> > > >>>>>> - install the google-java-format plugin, enable it
> > > >>>>>> - install the Save Actions plugin, enable "Optimize Imports" and
> > > >>>>>> "Reformat File"
> > > >>>>>>
> > > >>>>>> With this, every time you save, formatting will be applied
> > > >>>>>> automatically. You will never see formatting complaints from CI
> > > >>>>>> (except for cases where you break semantical checkstyle rules,
> > > >> such as
> > > >>>>>> using certain imports that we don't allow.).
> > > >>>>>>
> > > >>>>>> What do you think?
> > > >>>>>>
> > > >>>>>> Best,
> > > >>>>>> Aljoscha
> > > >>>>>>
> > > >>>>>> On 19.10.20 12:36, Aljoscha Krettek wrote:
> > > >>>>>>> I don't like checkstyle because it cannot be easily applied
> from
> > > >> the
> > > >>>>>>> commandline. I'm happy to learn otherwise, though. And I'd
> also be
> > > >>>>>>> very happy about alternative suggestions that can do that.
> > > >>>>>>>
> > > >>>>>>> Right now, I think Spotless is the most straightforward tool
> for
> > > >>>>>>> that. Also I don't care so much about what style we choose in
> the
> > > >>>>>>> end. (If we choose one). My main motivation is that we have a
> > > >> common,
> > > >>>>>>> strict style that can easily applied via tooling so that we no
> > > >> longer
> > > >>>>>>> need to comment on coding style in PRs.
> > > >>>>>>>
> > > >>>>>>> Aljoscha
> > > >>>>>>>
> > > >>>>>>> On 09.10.20 11:11, tison wrote:
> > > >>>>>>>> +1 on locking on one codestyle and I think that is what
> current
> > > >>>>>>>> checkstyle
> > > >>>>>>>> rules serving.
> > > >>>>>>>>
> > > >>>>>>>> For automatic applying part, we suggest developing by IDEA and
> > > >> with
> > > >>>>>>>> Checkstyle Plugin on IDEA applying checkstyle suggestion is
> also
> > > >>>>>>>> automatic.
> > > >>>>>>>>
> > > >>>>>>>> One short coming for spotless is that we can hardly adjust
> rules
> > > >> if
> > > >>>> the
> > > >>>>>>>> project has its own issues to overcome. IIRC only several
> > > >>>>>>>> pre-defined rules
> > > >>>>>>>> and a clumsy general regex substitution rule can be used.
> > > >>>>>>>>
> > > >>>>>>>> FYI my team started with spotless but ended up with checkstyle
> > > >> with
> > > >>>> few
> > > >>>>>>>> rules and Checkstyle Plugin for automation.
> > > >>>>>>>>
> > > >>>>>>>> Codestyle, in another perspective, is part of cognition of
> > > >> developers
> > > >>>>>>>> working in project, not something we just apply before pull
> > > >> request.
> > > >>>> No
> > > >>>>>>>> matter how much automation introduced, most of developers will
> > > >>>> converge
> > > >>>>>>>> working with the configured codestyle.
> > > >>>>>>>>
> > > >>>>>>>> Best,
> > > >>>>>>>> tison.
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>> Kostas Kloudas <[hidden email]> 于2020年10月7日周三 下午6:37写道：
> > > >>>>>>>>
> > > >>>>>>>>> Hi all,
> > > >>>>>>>>>
> > > >>>>>>>>> +1 for enforcing "a" codestyle using "a" tool.
> > > >>>>>>>>>
> > > >>>>>>>>> As the project grows both in terms of LOCs and contributors,
> > > >> this
> > > >>>>>>>>> becomes more and more important as it eliminates some
> potential
> > > >>>> points
> > > >>>>>>>>> of friction without any additional effort.
> > > >>>>>>>>>
> > > >>>>>>>>> From the discussion, I am leaning towards having a single
> > > >> commit
> > > >>>> with
> > > >>>>>>>>> all the codestyle-related changes. This will avoid sprinkling
> > > >>>>>>>>> refactoring commits all over the place for the next year or
> > > >> more.
> > > >>>> But
> > > >>>>>>>>> if this is the price to pay for having consensus on a tool,
> > > >> then I
> > > >>>> am
> > > >>>>>>>>> ok with gradual implementation. I believe that the value
> added
> > > >> by
> > > >>>>>>>>> having an automated process of enforcing a codestyle exceeds
> the
> > > >>>> cost
> > > >>>>>>>>> of the nuisance of gradual refactoring.
> > > >>>>>>>>>
> > > >>>>>>>>> As for the actual format, I like the google-java-format but
> > > >> again,
> > > >>>> if
> > > >>>>>>>>> the community agrees on a different one I would not oppose
> that
> > > >> (as
> > > >>>>>>>>> long as it does not use the same amount of indentation for
> > > >> method
> > > >>>> args
> > > >>>>>>>>> and method body :P).
> > > >>>>>>>>>
> > > >>>>>>>>> Cheers,
> > > >>>>>>>>> Kostas
> > > >>>>>>>>>
> > > >>>>>>>>> On Wed, Oct 7, 2020 at 10:26 AM Chesnay Schepler <
> > > >>>> [hidden email]>
> > > >>>>>>>>> wrote:
> > > >>>>>>>>>>
> > > >>>>>>>>>> To me, ratchet seems to combine the worst aspects of both
> > > >>>> approaches.
> > > >>>>>>>>>> You get disruptive changes, but only in singular files,
> > > >>>>>>>>>> for something mundane as a typo fix or import change, which
> > > >> would
> > > >>>> be
> > > >>>>>>>>>> annoying to keep separate from the actual functional changes
> > > >> in a
> > > >>>> PR.
> > > >>>>>>>>>>
> > > >>>>>>>>>> On 10/7/2020 10:04 AM, Matthias Pohl wrote:
> > > >>>>>>>>>>> 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
> > > >>>>>>>>>>> <[hidden email]>
> > > >>>>>>>>> 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 <
> > > >> [hidden email]
> > > >>>>>
> > > >>>>>>>>> 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
> > > >>>>>>>>>>>>>> <[hidden email]
> > > >>>>>>>>>>
> > > >>>>>>>>>>>>>> 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
> > > >>
> > >
> > >
>

[Tzu-Li (Gordon) Tai]

+1

On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu <[hidden email]> wrote:

> +1. It can be very helpful for future development. Thanks for driving this!
>
> Thanks,
> Zhu
>
> Yangze Guo <[hidden email]> 于2020年12月17日周四 下午2:48写道：
>
> > +1
> > Thanks for driving this!
> >
> > Best,
> > Yangze Guo
> >
> > On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman <[hidden email]> wrote:
> > >
> > > +1 it works really well in StateFun for quite some time.
> > >
> > > On Thursday, December 17, 2020, Wei Zhong <[hidden email]>
> > wrote:
> > >
> > > > +1 for the coding style automation. Thanks for driving this!
> > > >
> > > > Best,
> > > > Wei
> > > >
> > > > > 在 2020年12月17日，10:10，Xingbo Huang <[hidden email]> 写道：
> > > > >
> > > > > +1 asap. Spotless can greatly help us save the time of fixing
> > checkstyle
> > > > > errors.
> > > > >
> > > > > Best,
> > > > > Xingbo
> > > > >
> > > > > Kostas Kloudas <[hidden email]> 于2020年12月17日周四 上午4:14写道：
> > > > >
> > > > >> +1 asap from my side as well.
> > > > >>
> > > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <[hidden email]>
> > > > wrote:
> > > > >>>
> > > > >>> +1 asap.
> > > > >>>
> > > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> > [hidden email]>
> > > > >> wrote:
> > > > >>>
> > > > >>>> +1
> > > > >>>>
> > > > >>>> Thanks for driving this.
> > > > >>>>
> > > > >>>> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> > [hidden email]>
> > > > >>>> wrote:
> > > > >>>>
> > > > >>>>> +1 to set this up ASAP. Now's a good time to (finally) finalize
> > this
> > > > >>>>> effort with a new release cycle having started and
> > > > >> christmas/vacations
> > > > >>>>> being around the corner.
> > > > >>>>>
> > > > >>>>> On 12/16/2020 7:20 PM, Aljoscha Krettek wrote:
> > > > >>>>>> Let's try and conclude this discussion! I've prepared a PoC
> that
> > > > >> uses
> > > > >>>>>> Spotless with google-java-format to do the formatting:
> > > > >>>>>> https://github.com/aljoscha/flink/commits/flink-xxx-spotless
> > > > >>>>>>
> > > > >>>>>> To summarize from earlier discussion, the main benefits are:
> > > > >>>>>> - no more worrying about code style, both as reviewer and
> > > > >> implementer
> > > > >>>>>> - everyone can configure their IDE to auto-format, there will
> > > > >> never
> > > > >>>>>> be unrelated formatting changes
> > > > >>>>>>
> > > > >>>>>> Also, this uses git's blame.ignoreRevsFile to add a file that
> > tells
> > > > >>>>>> git blame/IntelliJ to ignore the refactoring commit. However,
> > you
> > > > >> need
> > > > >>>>>> to manually configure your git for that using:
> > > > >>>>>>
> > > > >>>>>> $ git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > > >>>>>>
> > > > >>>>>> You can check out the PoC, look at the code in an IDE, play
> > around
> > > > >>>>>> with blame/annotations.
> > > > >>>>>>
> > > > >>>>>> By the way, the coding style is not configurable, it’s the
> > Google
> > > > >> Java
> > > > >>>>>> Style, wich uses spaces for indentation. In an IDE or on
> github
> > the
> > > > >>>>>> code looks barely different from the previous formatting at a
> > first
> > > > >>>>>> glance.
> > > > >>>>>>
> > > > >>>>>> For IDE setup, I will add a guide in the README but it boils
> > down
> > > > >> to:
> > > > >>>>>>
> > > > >>>>>> - install the google-java-format plugin, enable it
> > > > >>>>>> - install the Save Actions plugin, enable "Optimize Imports"
> and
> > > > >>>>>> "Reformat File"
> > > > >>>>>>
> > > > >>>>>> With this, every time you save, formatting will be applied
> > > > >>>>>> automatically. You will never see formatting complaints from
> CI
> > > > >>>>>> (except for cases where you break semantical checkstyle rules,
> > > > >> such as
> > > > >>>>>> using certain imports that we don't allow.).
> > > > >>>>>>
> > > > >>>>>> What do you think?
> > > > >>>>>>
> > > > >>>>>> Best,
> > > > >>>>>> Aljoscha
> > > > >>>>>>
> > > > >>>>>> On 19.10.20 12:36, Aljoscha Krettek wrote:
> > > > >>>>>>> I don't like checkstyle because it cannot be easily applied
> > from
> > > > >> the
> > > > >>>>>>> commandline. I'm happy to learn otherwise, though. And I'd
> > also be
> > > > >>>>>>> very happy about alternative suggestions that can do that.
> > > > >>>>>>>
> > > > >>>>>>> Right now, I think Spotless is the most straightforward tool
> > for
> > > > >>>>>>> that. Also I don't care so much about what style we choose in
> > the
> > > > >>>>>>> end. (If we choose one). My main motivation is that we have a
> > > > >> common,
> > > > >>>>>>> strict style that can easily applied via tooling so that we
> no
> > > > >> longer
> > > > >>>>>>> need to comment on coding style in PRs.
> > > > >>>>>>>
> > > > >>>>>>> Aljoscha
> > > > >>>>>>>
> > > > >>>>>>> On 09.10.20 11:11, tison wrote:
> > > > >>>>>>>> +1 on locking on one codestyle and I think that is what
> > current
> > > > >>>>>>>> checkstyle
> > > > >>>>>>>> rules serving.
> > > > >>>>>>>>
> > > > >>>>>>>> For automatic applying part, we suggest developing by IDEA
> and
> > > > >> with
> > > > >>>>>>>> Checkstyle Plugin on IDEA applying checkstyle suggestion is
> > also
> > > > >>>>>>>> automatic.
> > > > >>>>>>>>
> > > > >>>>>>>> One short coming for spotless is that we can hardly adjust
> > rules
> > > > >> if
> > > > >>>> the
> > > > >>>>>>>> project has its own issues to overcome. IIRC only several
> > > > >>>>>>>> pre-defined rules
> > > > >>>>>>>> and a clumsy general regex substitution rule can be used.
> > > > >>>>>>>>
> > > > >>>>>>>> FYI my team started with spotless but ended up with
> checkstyle
> > > > >> with
> > > > >>>> few
> > > > >>>>>>>> rules and Checkstyle Plugin for automation.
> > > > >>>>>>>>
> > > > >>>>>>>> Codestyle, in another perspective, is part of cognition of
> > > > >> developers
> > > > >>>>>>>> working in project, not something we just apply before pull
> > > > >> request.
> > > > >>>> No
> > > > >>>>>>>> matter how much automation introduced, most of developers
> will
> > > > >>>> converge
> > > > >>>>>>>> working with the configured codestyle.
> > > > >>>>>>>>
> > > > >>>>>>>> Best,
> > > > >>>>>>>> tison.
> > > > >>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>> Kostas Kloudas <[hidden email]> 于2020年10月7日周三 下午6:37写道：
> > > > >>>>>>>>
> > > > >>>>>>>>> Hi all,
> > > > >>>>>>>>>
> > > > >>>>>>>>> +1 for enforcing "a" codestyle using "a" tool.
> > > > >>>>>>>>>
> > > > >>>>>>>>> As the project grows both in terms of LOCs and
> contributors,
> > > > >> this
> > > > >>>>>>>>> becomes more and more important as it eliminates some
> > potential
> > > > >>>> points
> > > > >>>>>>>>> of friction without any additional effort.
> > > > >>>>>>>>>
> > > > >>>>>>>>> From the discussion, I am leaning towards having a single
> > > > >> commit
> > > > >>>> with
> > > > >>>>>>>>> all the codestyle-related changes. This will avoid
> sprinkling
> > > > >>>>>>>>> refactoring commits all over the place for the next year or
> > > > >> more.
> > > > >>>> But
> > > > >>>>>>>>> if this is the price to pay for having consensus on a tool,
> > > > >> then I
> > > > >>>> am
> > > > >>>>>>>>> ok with gradual implementation. I believe that the value
> > added
> > > > >> by
> > > > >>>>>>>>> having an automated process of enforcing a codestyle
> exceeds
> > the
> > > > >>>> cost
> > > > >>>>>>>>> of the nuisance of gradual refactoring.
> > > > >>>>>>>>>
> > > > >>>>>>>>> As for the actual format, I like the google-java-format but
> > > > >> again,
> > > > >>>> if
> > > > >>>>>>>>> the community agrees on a different one I would not oppose
> > that
> > > > >> (as
> > > > >>>>>>>>> long as it does not use the same amount of indentation for
> > > > >> method
> > > > >>>> args
> > > > >>>>>>>>> and method body :P).
> > > > >>>>>>>>>
> > > > >>>>>>>>> Cheers,
> > > > >>>>>>>>> Kostas
> > > > >>>>>>>>>
> > > > >>>>>>>>> On Wed, Oct 7, 2020 at 10:26 AM Chesnay Schepler <
> > > > >>>> [hidden email]>
> > > > >>>>>>>>> wrote:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> To me, ratchet seems to combine the worst aspects of both
> > > > >>>> approaches.
> > > > >>>>>>>>>> You get disruptive changes, but only in singular files,
> > > > >>>>>>>>>> for something mundane as a typo fix or import change,
> which
> > > > >> would
> > > > >>>> be
> > > > >>>>>>>>>> annoying to keep separate from the actual functional
> changes
> > > > >> in a
> > > > >>>> PR.
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> On 10/7/2020 10:04 AM, Matthias Pohl wrote:
> > > > >>>>>>>>>>> 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
> > > > >>>>>>>>>>> <[hidden email]>
> > > > >>>>>>>>> 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 <
> > > > >> [hidden email]
> > > > >>>>>
> > > > >>>>>>>>> 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
> > > > >>>>>>>>>>>>>> <[hidden email]
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>>>>> 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
> > > > >>
> > > >
> > > >
> >
>

[Till Rohrmann]

+1

Cheers,
Till

On Thu, Dec 17, 2020 at 7:59 AM Tzu-Li (Gordon) Tai <[hidden email]>
wrote:

> +1
>
> On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu <[hidden email]> wrote:
>
> > +1. It can be very helpful for future development. Thanks for driving
> this!
> >
> > Thanks,
> > Zhu
> >
> > Yangze Guo <[hidden email]> 于2020年12月17日周四 下午2:48写道：
> >
> > > +1
> > > Thanks for driving this!
> > >
> > > Best,
> > > Yangze Guo
> > >
> > > On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman <[hidden email]>
> wrote:
> > > >
> > > > +1 it works really well in StateFun for quite some time.
> > > >
> > > > On Thursday, December 17, 2020, Wei Zhong <[hidden email]>
> > > wrote:
> > > >
> > > > > +1 for the coding style automation. Thanks for driving this!
> > > > >
> > > > > Best,
> > > > > Wei
> > > > >
> > > > > > 在 2020年12月17日，10:10，Xingbo Huang <[hidden email]> 写道：
> > > > > >
> > > > > > +1 asap. Spotless can greatly help us save the time of fixing
> > > checkstyle
> > > > > > errors.
> > > > > >
> > > > > > Best,
> > > > > > Xingbo
> > > > > >
> > > > > > Kostas Kloudas <[hidden email]> 于2020年12月17日周四 上午4:14写道：
> > > > > >
> > > > > >> +1 asap from my side as well.
> > > > > >>
> > > > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <
> [hidden email]>
> > > > > wrote:
> > > > > >>>
> > > > > >>> +1 asap.
> > > > > >>>
> > > > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> > > [hidden email]>
> > > > > >> wrote:
> > > > > >>>
> > > > > >>>> +1
> > > > > >>>>
> > > > > >>>> Thanks for driving this.
> > > > > >>>>
> > > > > >>>> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> > > [hidden email]>
> > > > > >>>> wrote:
> > > > > >>>>
> > > > > >>>>> +1 to set this up ASAP. Now's a good time to (finally)
> finalize
> > > this
> > > > > >>>>> effort with a new release cycle having started and
> > > > > >> christmas/vacations
> > > > > >>>>> being around the corner.
> > > > > >>>>>
> > > > > >>>>> On 12/16/2020 7:20 PM, Aljoscha Krettek wrote:
> > > > > >>>>>> Let's try and conclude this discussion! I've prepared a PoC
> > that
> > > > > >> uses
> > > > > >>>>>> Spotless with google-java-format to do the formatting:
> > > > > >>>>>>
> https://github.com/aljoscha/flink/commits/flink-xxx-spotless
> > > > > >>>>>>
> > > > > >>>>>> To summarize from earlier discussion, the main benefits are:
> > > > > >>>>>> - no more worrying about code style, both as reviewer and
> > > > > >> implementer
> > > > > >>>>>> - everyone can configure their IDE to auto-format, there
> will
> > > > > >> never
> > > > > >>>>>> be unrelated formatting changes
> > > > > >>>>>>
> > > > > >>>>>> Also, this uses git's blame.ignoreRevsFile to add a file
> that
> > > tells
> > > > > >>>>>> git blame/IntelliJ to ignore the refactoring commit.
> However,
> > > you
> > > > > >> need
> > > > > >>>>>> to manually configure your git for that using:
> > > > > >>>>>>
> > > > > >>>>>> $ git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > > > >>>>>>
> > > > > >>>>>> You can check out the PoC, look at the code in an IDE, play
> > > around
> > > > > >>>>>> with blame/annotations.
> > > > > >>>>>>
> > > > > >>>>>> By the way, the coding style is not configurable, it’s the
> > > Google
> > > > > >> Java
> > > > > >>>>>> Style, wich uses spaces for indentation. In an IDE or on
> > github
> > > the
> > > > > >>>>>> code looks barely different from the previous formatting at
> a
> > > first
> > > > > >>>>>> glance.
> > > > > >>>>>>
> > > > > >>>>>> For IDE setup, I will add a guide in the README but it boils
> > > down
> > > > > >> to:
> > > > > >>>>>>
> > > > > >>>>>> - install the google-java-format plugin, enable it
> > > > > >>>>>> - install the Save Actions plugin, enable "Optimize Imports"
> > and
> > > > > >>>>>> "Reformat File"
> > > > > >>>>>>
> > > > > >>>>>> With this, every time you save, formatting will be applied
> > > > > >>>>>> automatically. You will never see formatting complaints from
> > CI
> > > > > >>>>>> (except for cases where you break semantical checkstyle
> rules,
> > > > > >> such as
> > > > > >>>>>> using certain imports that we don't allow.).
> > > > > >>>>>>
> > > > > >>>>>> What do you think?
> > > > > >>>>>>
> > > > > >>>>>> Best,
> > > > > >>>>>> Aljoscha
> > > > > >>>>>>
> > > > > >>>>>> On 19.10.20 12:36, Aljoscha Krettek wrote:
> > > > > >>>>>>> I don't like checkstyle because it cannot be easily applied
> > > from
> > > > > >> the
> > > > > >>>>>>> commandline. I'm happy to learn otherwise, though. And I'd
> > > also be
> > > > > >>>>>>> very happy about alternative suggestions that can do that.
> > > > > >>>>>>>
> > > > > >>>>>>> Right now, I think Spotless is the most straightforward
> tool
> > > for
> > > > > >>>>>>> that. Also I don't care so much about what style we choose
> in
> > > the
> > > > > >>>>>>> end. (If we choose one). My main motivation is that we
> have a
> > > > > >> common,
> > > > > >>>>>>> strict style that can easily applied via tooling so that we
> > no
> > > > > >> longer
> > > > > >>>>>>> need to comment on coding style in PRs.
> > > > > >>>>>>>
> > > > > >>>>>>> Aljoscha
> > > > > >>>>>>>
> > > > > >>>>>>> On 09.10.20 11:11, tison wrote:
> > > > > >>>>>>>> +1 on locking on one codestyle and I think that is what
> > > current
> > > > > >>>>>>>> checkstyle
> > > > > >>>>>>>> rules serving.
> > > > > >>>>>>>>
> > > > > >>>>>>>> For automatic applying part, we suggest developing by IDEA
> > and
> > > > > >> with
> > > > > >>>>>>>> Checkstyle Plugin on IDEA applying checkstyle suggestion
> is
> > > also
> > > > > >>>>>>>> automatic.
> > > > > >>>>>>>>
> > > > > >>>>>>>> One short coming for spotless is that we can hardly adjust
> > > rules
> > > > > >> if
> > > > > >>>> the
> > > > > >>>>>>>> project has its own issues to overcome. IIRC only several
> > > > > >>>>>>>> pre-defined rules
> > > > > >>>>>>>> and a clumsy general regex substitution rule can be used.
> > > > > >>>>>>>>
> > > > > >>>>>>>> FYI my team started with spotless but ended up with
> > checkstyle
> > > > > >> with
> > > > > >>>> few
> > > > > >>>>>>>> rules and Checkstyle Plugin for automation.
> > > > > >>>>>>>>
> > > > > >>>>>>>> Codestyle, in another perspective, is part of cognition of
> > > > > >> developers
> > > > > >>>>>>>> working in project, not something we just apply before
> pull
> > > > > >> request.
> > > > > >>>> No
> > > > > >>>>>>>> matter how much automation introduced, most of developers
> > will
> > > > > >>>> converge
> > > > > >>>>>>>> working with the configured codestyle.
> > > > > >>>>>>>>
> > > > > >>>>>>>> Best,
> > > > > >>>>>>>> tison.
> > > > > >>>>>>>>
> > > > > >>>>>>>>
> > > > > >>>>>>>> Kostas Kloudas <[hidden email]> 于2020年10月7日周三
> 下午6:37写道：
> > > > > >>>>>>>>
> > > > > >>>>>>>>> Hi all,
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> +1 for enforcing "a" codestyle using "a" tool.
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> As the project grows both in terms of LOCs and
> > contributors,
> > > > > >> this
> > > > > >>>>>>>>> becomes more and more important as it eliminates some
> > > potential
> > > > > >>>> points
> > > > > >>>>>>>>> of friction without any additional effort.
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> From the discussion, I am leaning towards having a single
> > > > > >> commit
> > > > > >>>> with
> > > > > >>>>>>>>> all the codestyle-related changes. This will avoid
> > sprinkling
> > > > > >>>>>>>>> refactoring commits all over the place for the next year
> or
> > > > > >> more.
> > > > > >>>> But
> > > > > >>>>>>>>> if this is the price to pay for having consensus on a
> tool,
> > > > > >> then I
> > > > > >>>> am
> > > > > >>>>>>>>> ok with gradual implementation. I believe that the value
> > > added
> > > > > >> by
> > > > > >>>>>>>>> having an automated process of enforcing a codestyle
> > exceeds
> > > the
> > > > > >>>> cost
> > > > > >>>>>>>>> of the nuisance of gradual refactoring.
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> As for the actual format, I like the google-java-format
> but
> > > > > >> again,
> > > > > >>>> if
> > > > > >>>>>>>>> the community agrees on a different one I would not
> oppose
> > > that
> > > > > >> (as
> > > > > >>>>>>>>> long as it does not use the same amount of indentation
> for
> > > > > >> method
> > > > > >>>> args
> > > > > >>>>>>>>> and method body :P).
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> Cheers,
> > > > > >>>>>>>>> Kostas
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> On Wed, Oct 7, 2020 at 10:26 AM Chesnay Schepler <
> > > > > >>>> [hidden email]>
> > > > > >>>>>>>>> wrote:
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>> To me, ratchet seems to combine the worst aspects of
> both
> > > > > >>>> approaches.
> > > > > >>>>>>>>>> You get disruptive changes, but only in singular files,
> > > > > >>>>>>>>>> for something mundane as a typo fix or import change,
> > which
> > > > > >> would
> > > > > >>>> be
> > > > > >>>>>>>>>> annoying to keep separate from the actual functional
> > changes
> > > > > >> in a
> > > > > >>>> PR.
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>> On 10/7/2020 10:04 AM, Matthias Pohl wrote:
> > > > > >>>>>>>>>>> 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
> > > > > >>>>>>>>>>> <[hidden email]>
> > > > > >>>>>>>>> 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 <
> > > > > >> [hidden email]
> > > > > >>>>>
> > > > > >>>>>>>>> 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
> > > > > >>>>>>>>>>>>>> <[hidden email]
> > > > > >>>>>>>>>>
> > > > > >>>>>>>>>>>>>> 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
> > > > > >>
> > > > >
> > > > >
> > >
> >
>

[Piotr Nowojski-5]

Hi,

+1 for this kind of approach (with reformatting the code base).

Maybe ideally it would be best to do it either next week, or between
December 24th and January 1st? Maybe such timing would minimize the number
of open PRs that would need to be rewritten/modified to comply? I'm afraid
pending PRs would have huge conflicts that might need to be fixed manually.

Piotrek

czw., 17 gru 2020 o 09:58 Till Rohrmann <[hidden email]> napisał(a):

> +1
>
> Cheers,
> Till
>
> On Thu, Dec 17, 2020 at 7:59 AM Tzu-Li (Gordon) Tai <[hidden email]>
> wrote:
>
> > +1
> >
> > On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu <[hidden email]> wrote:
> >
> > > +1. It can be very helpful for future development. Thanks for driving
> > this!
> > >
> > > Thanks,
> > > Zhu
> > >
> > > Yangze Guo <[hidden email]> 于2020年12月17日周四 下午2:48写道：
> > >
> > > > +1
> > > > Thanks for driving this!
> > > >
> > > > Best,
> > > > Yangze Guo
> > > >
> > > > On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman <[hidden email]>
> > wrote:
> > > > >
> > > > > +1 it works really well in StateFun for quite some time.
> > > > >
> > > > > On Thursday, December 17, 2020, Wei Zhong <[hidden email]>
> > > > wrote:
> > > > >
> > > > > > +1 for the coding style automation. Thanks for driving this!
> > > > > >
> > > > > > Best,
> > > > > > Wei
> > > > > >
> > > > > > > 在 2020年12月17日，10:10，Xingbo Huang <[hidden email]> 写道：
> > > > > > >
> > > > > > > +1 asap. Spotless can greatly help us save the time of fixing
> > > > checkstyle
> > > > > > > errors.
> > > > > > >
> > > > > > > Best,
> > > > > > > Xingbo
> > > > > > >
> > > > > > > Kostas Kloudas <[hidden email]> 于2020年12月17日周四 上午4:14写道：
> > > > > > >
> > > > > > >> +1 asap from my side as well.
> > > > > > >>
> > > > > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <
> > [hidden email]>
> > > > > > wrote:
> > > > > > >>>
> > > > > > >>> +1 asap.
> > > > > > >>>
> > > > > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> > > > [hidden email]>
> > > > > > >> wrote:
> > > > > > >>>
> > > > > > >>>> +1
> > > > > > >>>>
> > > > > > >>>> Thanks for driving this.
> > > > > > >>>>
> > > > > > >>>> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> > > > [hidden email]>
> > > > > > >>>> wrote:
> > > > > > >>>>
> > > > > > >>>>> +1 to set this up ASAP. Now's a good time to (finally)
> > finalize
> > > > this
> > > > > > >>>>> effort with a new release cycle having started and
> > > > > > >> christmas/vacations
> > > > > > >>>>> being around the corner.
> > > > > > >>>>>
> > > > > > >>>>> On 12/16/2020 7:20 PM, Aljoscha Krettek wrote:
> > > > > > >>>>>> Let's try and conclude this discussion! I've prepared a
> PoC
> > > that
> > > > > > >> uses
> > > > > > >>>>>> Spotless with google-java-format to do the formatting:
> > > > > > >>>>>>
> > https://github.com/aljoscha/flink/commits/flink-xxx-spotless
> > > > > > >>>>>>
> > > > > > >>>>>> To summarize from earlier discussion, the main benefits
> are:
> > > > > > >>>>>> - no more worrying about code style, both as reviewer and
> > > > > > >> implementer
> > > > > > >>>>>> - everyone can configure their IDE to auto-format, there
> > will
> > > > > > >> never
> > > > > > >>>>>> be unrelated formatting changes
> > > > > > >>>>>>
> > > > > > >>>>>> Also, this uses git's blame.ignoreRevsFile to add a file
> > that
> > > > tells
> > > > > > >>>>>> git blame/IntelliJ to ignore the refactoring commit.
> > However,
> > > > you
> > > > > > >> need
> > > > > > >>>>>> to manually configure your git for that using:
> > > > > > >>>>>>
> > > > > > >>>>>> $ git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > > > > >>>>>>
> > > > > > >>>>>> You can check out the PoC, look at the code in an IDE,
> play
> > > > around
> > > > > > >>>>>> with blame/annotations.
> > > > > > >>>>>>
> > > > > > >>>>>> By the way, the coding style is not configurable, it’s the
> > > > Google
> > > > > > >> Java
> > > > > > >>>>>> Style, wich uses spaces for indentation. In an IDE or on
> > > github
> > > > the
> > > > > > >>>>>> code looks barely different from the previous formatting
> at
> > a
> > > > first
> > > > > > >>>>>> glance.
> > > > > > >>>>>>
> > > > > > >>>>>> For IDE setup, I will add a guide in the README but it
> boils
> > > > down
> > > > > > >> to:
> > > > > > >>>>>>
> > > > > > >>>>>> - install the google-java-format plugin, enable it
> > > > > > >>>>>> - install the Save Actions plugin, enable "Optimize
> Imports"
> > > and
> > > > > > >>>>>> "Reformat File"
> > > > > > >>>>>>
> > > > > > >>>>>> With this, every time you save, formatting will be applied
> > > > > > >>>>>> automatically. You will never see formatting complaints
> from
> > > CI
> > > > > > >>>>>> (except for cases where you break semantical checkstyle
> > rules,
> > > > > > >> such as
> > > > > > >>>>>> using certain imports that we don't allow.).
> > > > > > >>>>>>
> > > > > > >>>>>> What do you think?
> > > > > > >>>>>>
> > > > > > >>>>>> Best,
> > > > > > >>>>>> Aljoscha
> > > > > > >>>>>>
> > > > > > >>>>>> On 19.10.20 12:36, Aljoscha Krettek wrote:
> > > > > > >>>>>>> I don't like checkstyle because it cannot be easily
> applied
> > > > from
> > > > > > >> the
> > > > > > >>>>>>> commandline. I'm happy to learn otherwise, though. And
> I'd
> > > > also be
> > > > > > >>>>>>> very happy about alternative suggestions that can do
> that.
> > > > > > >>>>>>>
> > > > > > >>>>>>> Right now, I think Spotless is the most straightforward
> > tool
> > > > for
> > > > > > >>>>>>> that. Also I don't care so much about what style we
> choose
> > in
> > > > the
> > > > > > >>>>>>> end. (If we choose one). My main motivation is that we
> > have a
> > > > > > >> common,
> > > > > > >>>>>>> strict style that can easily applied via tooling so that
> we
> > > no
> > > > > > >> longer
> > > > > > >>>>>>> need to comment on coding style in PRs.
> > > > > > >>>>>>>
> > > > > > >>>>>>> Aljoscha
> > > > > > >>>>>>>
> > > > > > >>>>>>> On 09.10.20 11:11, tison wrote:
> > > > > > >>>>>>>> +1 on locking on one codestyle and I think that is what
> > > > current
> > > > > > >>>>>>>> checkstyle
> > > > > > >>>>>>>> rules serving.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> For automatic applying part, we suggest developing by
> IDEA
> > > and
> > > > > > >> with
> > > > > > >>>>>>>> Checkstyle Plugin on IDEA applying checkstyle suggestion
> > is
> > > > also
> > > > > > >>>>>>>> automatic.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> One short coming for spotless is that we can hardly
> adjust
> > > > rules
> > > > > > >> if
> > > > > > >>>> the
> > > > > > >>>>>>>> project has its own issues to overcome. IIRC only
> several
> > > > > > >>>>>>>> pre-defined rules
> > > > > > >>>>>>>> and a clumsy general regex substitution rule can be
> used.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> FYI my team started with spotless but ended up with
> > > checkstyle
> > > > > > >> with
> > > > > > >>>> few
> > > > > > >>>>>>>> rules and Checkstyle Plugin for automation.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> Codestyle, in another perspective, is part of cognition
> of
> > > > > > >> developers
> > > > > > >>>>>>>> working in project, not something we just apply before
> > pull
> > > > > > >> request.
> > > > > > >>>> No
> > > > > > >>>>>>>> matter how much automation introduced, most of
> developers
> > > will
> > > > > > >>>> converge
> > > > > > >>>>>>>> working with the configured codestyle.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> Best,
> > > > > > >>>>>>>> tison.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> Kostas Kloudas <[hidden email]> 于2020年10月7日周三
> > 下午6:37写道：
> > > > > > >>>>>>>>
> > > > > > >>>>>>>>> Hi all,
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> +1 for enforcing "a" codestyle using "a" tool.
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> As the project grows both in terms of LOCs and
> > > contributors,
> > > > > > >> this
> > > > > > >>>>>>>>> becomes more and more important as it eliminates some
> > > > potential
> > > > > > >>>> points
> > > > > > >>>>>>>>> of friction without any additional effort.
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> From the discussion, I am leaning towards having a
> single
> > > > > > >> commit
> > > > > > >>>> with
> > > > > > >>>>>>>>> all the codestyle-related changes. This will avoid
> > > sprinkling
> > > > > > >>>>>>>>> refactoring commits all over the place for the next
> year
> > or
> > > > > > >> more.
> > > > > > >>>> But
> > > > > > >>>>>>>>> if this is the price to pay for having consensus on a
> > tool,
> > > > > > >> then I
> > > > > > >>>> am
> > > > > > >>>>>>>>> ok with gradual implementation. I believe that the
> value
> > > > added
> > > > > > >> by
> > > > > > >>>>>>>>> having an automated process of enforcing a codestyle
> > > exceeds
> > > > the
> > > > > > >>>> cost
> > > > > > >>>>>>>>> of the nuisance of gradual refactoring.
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> As for the actual format, I like the google-java-format
> > but
> > > > > > >> again,
> > > > > > >>>> if
> > > > > > >>>>>>>>> the community agrees on a different one I would not
> > oppose
> > > > that
> > > > > > >> (as
> > > > > > >>>>>>>>> long as it does not use the same amount of indentation
> > for
> > > > > > >> method
> > > > > > >>>> args
> > > > > > >>>>>>>>> and method body :P).
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> Cheers,
> > > > > > >>>>>>>>> Kostas
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> On Wed, Oct 7, 2020 at 10:26 AM Chesnay Schepler <
> > > > > > >>>> [hidden email]>
> > > > > > >>>>>>>>> wrote:
> > > > > > >>>>>>>>>>
> > > > > > >>>>>>>>>> To me, ratchet seems to combine the worst aspects of
> > both
> > > > > > >>>> approaches.
> > > > > > >>>>>>>>>> You get disruptive changes, but only in singular
> files,
> > > > > > >>>>>>>>>> for something mundane as a typo fix or import change,
> > > which
> > > > > > >> would
> > > > > > >>>> be
> > > > > > >>>>>>>>>> annoying to keep separate from the actual functional
> > > changes
> > > > > > >> in a
> > > > > > >>>> PR.
> > > > > > >>>>>>>>>>
> > > > > > >>>>>>>>>> On 10/7/2020 10:04 AM, Matthias Pohl wrote:
> > > > > > >>>>>>>>>>> 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
> > > > > > >>>>>>>>>>> <[hidden email]>
> > > > > > >>>>>>>>> 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 <
> > > > > > >> [hidden email]
> > > > > > >>>>>
> > > > > > >>>>>>>>> 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
> > > > > > >>>>>>>>>>>>>> <[hidden email]
> > > > > > >>>>>>>>>>
> > > > > > >>>>>>>>>>>>>> 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
> > > > > > >>
> > > > > >
> > > > > >
> > > >
> > >
> >
>

[Aljoscha Krettek-2]

Thanks for the broad support! I created a Jira issue:
https://issues.apache.org/jira/browse/FLINK-20651

For the timing, I agree with Piotr.

Best,
Aljoscha

On 17.12.20 11:45, Piotr Nowojski wrote:

> Hi,
>
> +1 for this kind of approach (with reformatting the code base).
>
> Maybe ideally it would be best to do it either next week, or between
> December 24th and January 1st? Maybe such timing would minimize the number
> of open PRs that would need to be rewritten/modified to comply? I'm afraid
> pending PRs would have huge conflicts that might need to be fixed manually.
>
> Piotrek
>
> czw., 17 gru 2020 o 09:58 Till Rohrmann <[hidden email]> napisał(a):
>
>> +1
>>
>> Cheers,
>> Till
>>
>> On Thu, Dec 17, 2020 at 7:59 AM Tzu-Li (Gordon) Tai <[hidden email]>
>> wrote:
>>
>>> +1
>>>
>>> On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu <[hidden email]> wrote:
>>>
>>>> +1. It can be very helpful for future development. Thanks for driving
>>> this!
>>>>
>>>> Thanks,
>>>> Zhu
>>>>
>>>> Yangze Guo <[hidden email]> 于2020年12月17日周四 下午2:48写道：
>>>>
>>>>> +1
>>>>> Thanks for driving this!
>>>>>
>>>>> Best,
>>>>> Yangze Guo
>>>>>
>>>>> On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman <[hidden email]>
>>> wrote:
>>>>>>
>>>>>> +1 it works really well in StateFun for quite some time.
>>>>>>
>>>>>> On Thursday, December 17, 2020, Wei Zhong <[hidden email]>
>>>>> wrote:
>>>>>>
>>>>>>> +1 for the coding style automation. Thanks for driving this!
>>>>>>>
>>>>>>> Best,
>>>>>>> Wei
>>>>>>>
>>>>>>>> 在 2020年12月17日，10:10，Xingbo Huang <[hidden email]> 写道：
>>>>>>>>
>>>>>>>> +1 asap. Spotless can greatly help us save the time of fixing
>>>>> checkstyle
>>>>>>>> errors.
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> Xingbo
>>>>>>>>
>>>>>>>> Kostas Kloudas <[hidden email]> 于2020年12月17日周四 上午4:14写道：
>>>>>>>>
>>>>>>>>> +1 asap from my side as well.
>>>>>>>>>
>>>>>>>>> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <
>>> [hidden email]>
>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> +1 asap.
>>>>>>>>>>
>>>>>>>>>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
>>>>> [hidden email]>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> +1
>>>>>>>>>>>
>>>>>>>>>>> Thanks for driving this.
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
>>>>> [hidden email]>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> +1 to set this up ASAP. Now's a good time to (finally)
>>> finalize
>>>>> this
>>>>>>>>>>>> effort with a new release cycle having started and
>>>>>>>>> christmas/vacations
>>>>>>>>>>>> being around the corner.
>>>>>>>>>>>>
>>>>>>>>>>>> On 12/16/2020 7:20 PM, Aljoscha Krettek wrote:
>>>>>>>>>>>>> Let's try and conclude this discussion! I've prepared a
>> PoC
>>>> that
>>>>>>>>> uses
>>>>>>>>>>>>> Spotless with google-java-format to do the formatting:
>>>>>>>>>>>>>
>>> https://github.com/aljoscha/flink/commits/flink-xxx-spotless
>>>>>>>>>>>>>
>>>>>>>>>>>>> To summarize from earlier discussion, the main benefits
>> are:
>>>>>>>>>>>>> - no more worrying about code style, both as reviewer and
>>>>>>>>> implementer
>>>>>>>>>>>>> - everyone can configure their IDE to auto-format, there
>>> will
>>>>>>>>> never
>>>>>>>>>>>>> be unrelated formatting changes
>>>>>>>>>>>>>
>>>>>>>>>>>>> Also, this uses git's blame.ignoreRevsFile to add a file
>>> that
>>>>> tells
>>>>>>>>>>>>> git blame/IntelliJ to ignore the refactoring commit.
>>> However,
>>>>> you
>>>>>>>>> need
>>>>>>>>>>>>> to manually configure your git for that using:
>>>>>>>>>>>>>
>>>>>>>>>>>>> $ git config blame.ignoreRevsFile .git-blame-ignore-revs
>>>>>>>>>>>>>
>>>>>>>>>>>>> You can check out the PoC, look at the code in an IDE,
>> play
>>>>> around
>>>>>>>>>>>>> with blame/annotations.
>>>>>>>>>>>>>
>>>>>>>>>>>>> By the way, the coding style is not configurable, it’s the
>>>>> Google
>>>>>>>>> Java
>>>>>>>>>>>>> Style, wich uses spaces for indentation. In an IDE or on
>>>> github
>>>>> the
>>>>>>>>>>>>> code looks barely different from the previous formatting
>> at
>>> a
>>>>> first
>>>>>>>>>>>>> glance.
>>>>>>>>>>>>>
>>>>>>>>>>>>> For IDE setup, I will add a guide in the README but it
>> boils
>>>>> down
>>>>>>>>> to:
>>>>>>>>>>>>>
>>>>>>>>>>>>> - install the google-java-format plugin, enable it
>>>>>>>>>>>>> - install the Save Actions plugin, enable "Optimize
>> Imports"
>>>> and
>>>>>>>>>>>>> "Reformat File"
>>>>>>>>>>>>>
>>>>>>>>>>>>> With this, every time you save, formatting will be applied
>>>>>>>>>>>>> automatically. You will never see formatting complaints
>> from
>>>> CI
>>>>>>>>>>>>> (except for cases where you break semantical checkstyle
>>> rules,
>>>>>>>>> such as
>>>>>>>>>>>>> using certain imports that we don't allow.).
>>>>>>>>>>>>>
>>>>>>>>>>>>> What do you think?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best,
>>>>>>>>>>>>> Aljoscha
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 19.10.20 12:36, Aljoscha Krettek wrote:
>>>>>>>>>>>>>> I don't like checkstyle because it cannot be easily
>> applied
>>>>> from
>>>>>>>>> the
>>>>>>>>>>>>>> commandline. I'm happy to learn otherwise, though. And
>> I'd
>>>>> also be
>>>>>>>>>>>>>> very happy about alternative suggestions that can do
>> that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Right now, I think Spotless is the most straightforward
>>> tool
>>>>> for
>>>>>>>>>>>>>> that. Also I don't care so much about what style we
>> choose
>>> in
>>>>> the
>>>>>>>>>>>>>> end. (If we choose one). My main motivation is that we
>>> have a
>>>>>>>>> common,
>>>>>>>>>>>>>> strict style that can easily applied via tooling so that
>> we
>>>> no
>>>>>>>>> longer
>>>>>>>>>>>>>> need to comment on coding style in PRs.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Aljoscha
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 09.10.20 11:11, tison wrote:
>>>>>>>>>>>>>>> +1 on locking on one codestyle and I think that is what
>>>>> current
>>>>>>>>>>>>>>> checkstyle
>>>>>>>>>>>>>>> rules serving.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> For automatic applying part, we suggest developing by
>> IDEA
>>>> and
>>>>>>>>> with
>>>>>>>>>>>>>>> Checkstyle Plugin on IDEA applying checkstyle suggestion
>>> is
>>>>> also
>>>>>>>>>>>>>>> automatic.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> One short coming for spotless is that we can hardly
>> adjust
>>>>> rules
>>>>>>>>> if
>>>>>>>>>>> the
>>>>>>>>>>>>>>> project has its own issues to overcome. IIRC only
>> several
>>>>>>>>>>>>>>> pre-defined rules
>>>>>>>>>>>>>>> and a clumsy general regex substitution rule can be
>> used.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> FYI my team started with spotless but ended up with
>>>> checkstyle
>>>>>>>>> with
>>>>>>>>>>> few
>>>>>>>>>>>>>>> rules and Checkstyle Plugin for automation.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Codestyle, in another perspective, is part of cognition
>> of
>>>>>>>>> developers
>>>>>>>>>>>>>>> working in project, not something we just apply before
>>> pull
>>>>>>>>> request.
>>>>>>>>>>> No
>>>>>>>>>>>>>>> matter how much automation introduced, most of
>> developers
>>>> will
>>>>>>>>>>> converge
>>>>>>>>>>>>>>> working with the configured codestyle.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Best,
>>>>>>>>>>>>>>> tison.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Kostas Kloudas <[hidden email]> 于2020年10月7日周三
>>> 下午6:37写道：
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> +1 for enforcing "a" codestyle using "a" tool.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> As the project grows both in terms of LOCs and
>>>> contributors,
>>>>>>>>> this
>>>>>>>>>>>>>>>> becomes more and more important as it eliminates some
>>>>> potential
>>>>>>>>>>> points
>>>>>>>>>>>>>>>> of friction without any additional effort.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>  From the discussion, I am leaning towards having a
>> single
>>>>>>>>> commit
>>>>>>>>>>> with
>>>>>>>>>>>>>>>> all the codestyle-related changes. This will avoid
>>>> sprinkling
>>>>>>>>>>>>>>>> refactoring commits all over the place for the next
>> year
>>> or
>>>>>>>>> more.
>>>>>>>>>>> But
>>>>>>>>>>>>>>>> if this is the price to pay for having consensus on a
>>> tool,
>>>>>>>>> then I
>>>>>>>>>>> am
>>>>>>>>>>>>>>>> ok with gradual implementation. I believe that the
>> value
>>>>> added
>>>>>>>>> by
>>>>>>>>>>>>>>>> having an automated process of enforcing a codestyle
>>>> exceeds
>>>>> the
>>>>>>>>>>> cost
>>>>>>>>>>>>>>>> of the nuisance of gradual refactoring.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> As for the actual format, I like the google-java-format
>>> but
>>>>>>>>> again,
>>>>>>>>>>> if
>>>>>>>>>>>>>>>> the community agrees on a different one I would not
>>> oppose
>>>>> that
>>>>>>>>> (as
>>>>>>>>>>>>>>>> long as it does not use the same amount of indentation
>>> for
>>>>>>>>> method
>>>>>>>>>>> args
>>>>>>>>>>>>>>>> and method body :P).
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>> Kostas
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Wed, Oct 7, 2020 at 10:26 AM Chesnay Schepler <
>>>>>>>>>>> [hidden email]>
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> To me, ratchet seems to combine the worst aspects of
>>> both
>>>>>>>>>>> approaches.
>>>>>>>>>>>>>>>>> You get disruptive changes, but only in singular
>> files,
>>>>>>>>>>>>>>>>> for something mundane as a typo fix or import change,
>>>> which
>>>>>>>>> would
>>>>>>>>>>> be
>>>>>>>>>>>>>>>>> annoying to keep separate from the actual functional
>>>> changes
>>>>>>>>> in a
>>>>>>>>>>> PR.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 10/7/2020 10:04 AM, Matthias Pohl wrote:
>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>>>>>>> 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 <
>>>>>>>>> [hidden email]
>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>>>>> <[hidden email]
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>>
>