[DISCUSS] Enforce common opinionated coding style using Spotless

[Aljoscha Krettek]

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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Flavio Pompermaier]

Hi Aljoscha,
I think that having the style check directly in the IDE is a very good
feature so +1 on my side as a contributor (I also asked once on the mailing
list if there was already something like that)..I never used Spotless so I
can't say if it easy to integrate with the IDE but the nice thing is that
is has plugins both for Eclipse and IntelliJ so it was already on my
wish-to-try list ;)

Bye,
Flavio

On Tue, Oct 6, 2020 at 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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Chesnay Schepler]

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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Arvid Heise]

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  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



Follow us @VervericaData

--

Join Flink Forward  - 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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Arvid Heise]

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  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 
> 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
>
> 
>
> Follow us @VervericaData
>
> --
>
> Join Flink Forward  - The Apache Flink
> Conference
>
> Stream Processing | 

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Aljoscha Krettek]

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  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 
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 Intel

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Matthias Pohl]

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  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  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 
> >> 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-for

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Chesnay Schepler]

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  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  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 
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
"st

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Kostas Kloudas]

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  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  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  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 
>  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
> >>

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[tison]

+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  于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 
> 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 
> 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 
> wrote:
> > >>>
> >  I'm also +1 for automatically enforceable code 

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Aljoscha Krettek]

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  于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 
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 

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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Aljoscha Krettek]

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  于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 
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 Tu

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Chesnay Schepler]

+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  于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 
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 previo

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Robert Metzger]

+1

Thanks for driving this.

On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler  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  于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 
>  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
> > an

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Thomas Weise]

+1 for better coding style automation

I see spotless works very well in other projects.

On Wed, Dec 16, 2020 at 10:45 AM Robert Metzger  wrote:

> +1
>
> Thanks for driving this.
>
> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
> 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  于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 metho

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Arvid Heise]

+1 asap.

On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger  wrote:

> +1
>
> Thanks for driving this.
>
> On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
> 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  于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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Kostas Kloudas]

+1 asap from my side as well.

On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise  wrote:
>
> +1 asap.
>
> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger  wrote:
>
> > +1
> >
> > Thanks for driving this.
> >
> > On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
> > 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  于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

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Xingbo Huang]

+1 asap. Spotless can greatly help us save the time of fixing checkstyle
errors.

Best,
Xingbo

Kostas Kloudas  于2020年12月17日周四 上午4:14写道：

> +1 asap from my side as well.
>
> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise  wrote:
> >
> > +1 asap.
> >
> > On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger 
> wrote:
> >
> > > +1
> > >
> > > Thanks for driving this.
> > >
> > > On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
> > > 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  于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. T

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Wei Zhong]

+1 for the coding style automation. Thanks for driving this!

Best,
Wei

> 在 2020年12月17日，10:10，Xingbo Huang  写道：
> 
> +1 asap. Spotless can greatly help us save the time of fixing checkstyle
> errors.
> 
> Best,
> Xingbo
> 
> Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> 
>> +1 asap from my side as well.
>> 
>> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise  wrote:
>>> 
>>> +1 asap.
>>> 
>>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger 
>> wrote:
>>> 
 +1
 
 Thanks for driving this.
 
 On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
 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  于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.
 B

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Igal Shilman]

+1 it works really well in StateFun for quite some time.

On Thursday, December 17, 2020, Wei Zhong  wrote:

> +1 for the coding style automation. Thanks for driving this!
>
> Best,
> Wei
>
> > 在 2020年12月17日，10:10，Xingbo Huang  写道：
> >
> > +1 asap. Spotless can greatly help us save the time of fixing checkstyle
> > errors.
> >
> > Best,
> > Xingbo
> >
> > Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> >
> >> +1 asap from my side as well.
> >>
> >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise 
> wrote:
> >>>
> >>> +1 asap.
> >>>
> >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger 
> >> wrote:
> >>>
>  +1
> 
>  Thanks for driving this.
> 
>  On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
>  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  于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 impo

Re: [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  wrote:
>
> +1 it works really well in StateFun for quite some time.
>
> On Thursday, December 17, 2020, Wei Zhong  wrote:
>
> > +1 for the coding style automation. Thanks for driving this!
> >
> > Best,
> > Wei
> >
> > > 在 2020年12月17日，10:10，Xingbo Huang  写道：
> > >
> > > +1 asap. Spotless can greatly help us save the time of fixing checkstyle
> > > errors.
> > >
> > > Best,
> > > Xingbo
> > >
> > > Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> > >
> > >> +1 asap from my side as well.
> > >>
> > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise 
> > wrote:
> > >>>
> > >>> +1 asap.
> > >>>
> > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger 
> > >> wrote:
> > >>>
> >  +1
> > 
> >  Thanks for driving this.
> > 
> >  On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler 
> >  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 configur

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Zhu Zhu]

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

Thanks,
Zhu

Yangze Guo  于2020年12月17日周四 下午2:48写道：

> +1
> Thanks for driving this!
>
> Best,
> Yangze Guo
>
> On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman  wrote:
> >
> > +1 it works really well in StateFun for quite some time.
> >
> > On Thursday, December 17, 2020, Wei Zhong 
> wrote:
> >
> > > +1 for the coding style automation. Thanks for driving this!
> > >
> > > Best,
> > > Wei
> > >
> > > > 在 2020年12月17日，10:10，Xingbo Huang  写道：
> > > >
> > > > +1 asap. Spotless can greatly help us save the time of fixing
> checkstyle
> > > > errors.
> > > >
> > > > Best,
> > > > Xingbo
> > > >
> > > > Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> > > >
> > > >> +1 asap from my side as well.
> > > >>
> > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise 
> > > wrote:
> > > >>>
> > > >>> +1 asap.
> > > >>>
> > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> rmetz...@apache.org>
> > > >> wrote:
> > > >>>
> > >  +1
> > > 
> > >  Thanks for driving this.
> > > 
> > >  On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> ches...@apache.org>
> > >  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 st

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Tzu-Li (Gordon) Tai]

+1

On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu  wrote:

> +1. It can be very helpful for future development. Thanks for driving this!
>
> Thanks,
> Zhu
>
> Yangze Guo  于2020年12月17日周四 下午2:48写道：
>
> > +1
> > Thanks for driving this!
> >
> > Best,
> > Yangze Guo
> >
> > On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman  wrote:
> > >
> > > +1 it works really well in StateFun for quite some time.
> > >
> > > On Thursday, December 17, 2020, Wei Zhong 
> > wrote:
> > >
> > > > +1 for the coding style automation. Thanks for driving this!
> > > >
> > > > Best,
> > > > Wei
> > > >
> > > > > 在 2020年12月17日，10:10，Xingbo Huang  写道：
> > > > >
> > > > > +1 asap. Spotless can greatly help us save the time of fixing
> > checkstyle
> > > > > errors.
> > > > >
> > > > > Best,
> > > > > Xingbo
> > > > >
> > > > > Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> > > > >
> > > > >> +1 asap from my side as well.
> > > > >>
> > > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise 
> > > > wrote:
> > > > >>>
> > > > >>> +1 asap.
> > > > >>>
> > > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> > rmetz...@apache.org>
> > > > >> wrote:
> > > > >>>
> > > >  +1
> > > > 
> > > >  Thanks for driving this.
> > > > 
> > > >  On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> > ches...@apache.org>
> > > >  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
> > > >  automa

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Till Rohrmann]

+1

Cheers,
Till

On Thu, Dec 17, 2020 at 7:59 AM Tzu-Li (Gordon) Tai 
wrote:

> +1
>
> On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu  wrote:
>
> > +1. It can be very helpful for future development. Thanks for driving
> this!
> >
> > Thanks,
> > Zhu
> >
> > Yangze Guo  于2020年12月17日周四 下午2:48写道：
> >
> > > +1
> > > Thanks for driving this!
> > >
> > > Best,
> > > Yangze Guo
> > >
> > > On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman 
> wrote:
> > > >
> > > > +1 it works really well in StateFun for quite some time.
> > > >
> > > > On Thursday, December 17, 2020, Wei Zhong 
> > > wrote:
> > > >
> > > > > +1 for the coding style automation. Thanks for driving this!
> > > > >
> > > > > Best,
> > > > > Wei
> > > > >
> > > > > > 在 2020年12月17日，10:10，Xingbo Huang  写道：
> > > > > >
> > > > > > +1 asap. Spotless can greatly help us save the time of fixing
> > > checkstyle
> > > > > > errors.
> > > > > >
> > > > > > Best,
> > > > > > Xingbo
> > > > > >
> > > > > > Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> > > > > >
> > > > > >> +1 asap from my side as well.
> > > > > >>
> > > > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <
> ar...@ververica.com>
> > > > > wrote:
> > > > > >>>
> > > > > >>> +1 asap.
> > > > > >>>
> > > > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> > > rmetz...@apache.org>
> > > > > >> wrote:
> > > > > >>>
> > > > >  +1
> > > > > 
> > > > >  Thanks for driving this.
> > > > > 
> > > > >  On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> > > ches...@apache.org>
> > > > >  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
> > > > > >>>
> > 

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Piotr Nowojski]

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  napisał(a):

> +1
>
> Cheers,
> Till
>
> On Thu, Dec 17, 2020 at 7:59 AM Tzu-Li (Gordon) Tai 
> wrote:
>
> > +1
> >
> > On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu  wrote:
> >
> > > +1. It can be very helpful for future development. Thanks for driving
> > this!
> > >
> > > Thanks,
> > > Zhu
> > >
> > > Yangze Guo  于2020年12月17日周四 下午2:48写道：
> > >
> > > > +1
> > > > Thanks for driving this!
> > > >
> > > > Best,
> > > > Yangze Guo
> > > >
> > > > On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman 
> > wrote:
> > > > >
> > > > > +1 it works really well in StateFun for quite some time.
> > > > >
> > > > > On Thursday, December 17, 2020, Wei Zhong 
> > > > wrote:
> > > > >
> > > > > > +1 for the coding style automation. Thanks for driving this!
> > > > > >
> > > > > > Best,
> > > > > > Wei
> > > > > >
> > > > > > > 在 2020年12月17日，10:10，Xingbo Huang  写道：
> > > > > > >
> > > > > > > +1 asap. Spotless can greatly help us save the time of fixing
> > > > checkstyle
> > > > > > > errors.
> > > > > > >
> > > > > > > Best,
> > > > > > > Xingbo
> > > > > > >
> > > > > > > Kostas Kloudas  于2020年12月17日周四 上午4:14写道：
> > > > > > >
> > > > > > >> +1 asap from my side as well.
> > > > > > >>
> > > > > > >> On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <
> > ar...@ververica.com>
> > > > > > wrote:
> > > > > > >>>
> > > > > > >>> +1 asap.
> > > > > > >>>
> > > > > > >>> On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <
> > > > rmetz...@apache.org>
> > > > > > >> wrote:
> > > > > > >>>
> > > > > >  +1
> > > > > > 
> > > > > >  Thanks for driving this.
> > > > > > 
> > > > > >  On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <
> > > > ches...@apache.org>
> > > > > >  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
> 

Re: [DISCUSS] Enforce common opinionated coding style using Spotless

[Aljoscha Krettek]

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  napisał(a):


+1

Cheers,
Till

On Thu, Dec 17, 2020 at 7:59 AM Tzu-Li (Gordon) Tai 
wrote:


+1

On Thu, Dec 17, 2020, 2:56 PM Zhu Zhu  wrote:


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

this!


Thanks,
Zhu

Yangze Guo  于2020年12月17日周四 下午2:48写道：


+1
Thanks for driving this!

Best,
Yangze Guo

On Thu, Dec 17, 2020 at 2:39 PM Igal Shilman 

wrote:


+1 it works really well in StateFun for quite some time.

On Thursday, December 17, 2020, Wei Zhong 

wrote:



+1 for the coding style automation. Thanks for driving this!

Best,
Wei


在 2020年12月17日，10:10，Xingbo Huang  写道：

+1 asap. Spotless can greatly help us save the time of fixing

checkstyle

errors.

Best,
Xingbo

Kostas Kloudas  于2020年12月17日周四 上午4:14写道：


+1 asap from my side as well.

On Wed, Dec 16, 2020 at 8:04 PM Arvid Heise <

ar...@ververica.com>

wrote:


+1 asap.

On Wed, Dec 16, 2020 at 7:44 PM Robert Metzger <

rmetz...@apache.org>

wrote:



+1

Thanks for driving this.

On Wed, Dec 16, 2020 at 7:33 PM Chesnay Schepler <

ches...@apache.org>

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  于2020年10月7日周三

下午6:37写道：



Hi all,

+1 for enforcing "a" codestyle using "a" tool.

As the project grows bo