Re: [ANNOUNCE] New formatting rules are now in effect

[Matthias Pohl]

// With this one I am curious now how many of you had the same issue
without complaining: In the engine team there were 4 out of 4.

It's about the error dialog that pops up when creating a new Java class
file. Additionally, the Java class is generated but is not formatted
correctly. Reformatting the file helps. Confirming the dialog is annoying,
though. I started looking into the issue and found an
UnsupportedOperationException that led me to the actual cause: A bug in the
google-java-format plugin version 1.7.0.5 is causing this behavior. I
provided a more detailed description of my findings in FLINK-21106 [1]
including a compiled version of the patched plugin.

I want to open the discussion on how we want to deal with it. I see three
options right now:
1. We leave it as it is right now as we consider this to be a minor thing.
2. We provide the patched google-java-format plugin as part of the docs.
3. We upgrade to Java 11 to be able to upgrade the google-java-format
plugin as it was already mentioned earlier in the thread.

None of the above options seem to be the right one to go for. Any thoughts
on this?

Best,
Matthias

[1] https://issues.apache.org/jira/browse/FLINK-21106



On Wed, Dec 30, 2020 at 11:05 AM Chesnay Schepler 
wrote:

> 1) No, it is not possible to exclude certain code blocks from formatting.
> There is a workaround though for this case; you can add en empty comment
> (//) to the end of a line to prevent subsequent lines from being added
> to it.
> https://github.com/google/google-java-format/issues/137
>
> Note that any spotless-specific features would like not help us anyway,
> unless we'd be fine with not using the IntelliJ plugin.
>
> 2) The code style has not been updated yet.
> The indent choices with the google-java-format is either 2 spaces for
> everything, or 4 spaces + 8 spaces for continuations.
> In other words, 4 spaces is simply not an option.
>
> On 12/30/2020 9:09 AM, Jark Wu wrote:
> > Hi,
> >
> > I have played with the format plugin these days and found some problems.
> > Maybe some of them are personal taste.
> >
> > 1) Is it possible to disable auto-format for some code blocks?
> > For example, the format of code generation
> > StructuredObjectConverter#generateCode is manually
> >   adjusted for readability. However, the format plugin breaks it and it's
> > hard to read now.
> > See before[1] and after[2]. cc @Timo Walther 
> >
> > 2) Using 4 spaces or 8 spaces for continuation indent?
> > AOSP uses 8 spaces for continuation indent.
> > However, Flink Code Style suggests "Each new line should have one extra
> > indentation relative to
> > the line of the called entity" which means 4 spaces.
> > Personally, I think 4 spaces may be more friendly for Java lambdas.  An
> > example:
> >
> > 8 spaces:
> >
> > wrapClassLoader(
> >  () ->
> >  environment
> >  .getModules()
> >  .forEach(
> >  (name, entry) ->
> >  modules.put(
> >  name,
> >  createModule(
> >  entry.asMap(),
> > classLoader;
> >
> > 4 spaces:
> >
> > wrapClassLoader(
> >  () ->
> >  environment
> >  .getModules()
> >  .forEach(
> >  (name, entry) ->
> >  modules.put(name, createModule(entry.asMap(),
> > classLoader;
> >
> >
> >
> > Best,
> > Jark
> >
> > [1]:
> >
> https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L169
> > [2]:
> >
> https://github.com/apache/flink/blob/master/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L170
> > [3]:
> >
> https://flink.apache.org/contributing/code-style-and-quality-formatting.html#breaking-the-lines-of-too-long-statements
> >
> > On Tue, 29 Dec 2020 at 19:19, Chesnay Schepler 
> wrote:
> >
> >> I have filed FLINK-20803
> >>  for the issue that
> >> Till raised.
> >>
> >> google-java-format 1.8 and above require Java 11 to /run/, so we'll
> >> stick with 1.7 for the time being.
> >> This requires a manual installation of the google-java-format plugin,
> >> and remembering to not update the plugin (urgh...).
> >>
> >> Long term we may want to think about requiring Java 11 for /development/
> >> (*not* usage) of Flink.
> >>
> >> On 12/29/2020 12:13 PM, Till Rohrmann wrote:
> >>> I might have found a problem:
> >>>
> >>> In my current setup I am using the google-java-format plugin version
> >>> 1.9.0.0 which uses google-java-format 1.9.0. In our spotless
> >> configuration
> >>> we are using google-java-format 1.7.0, however. The

Re: [ANNOUNCE] New formatting rules are now in effect

[Jark Wu]

Hi Matthias,

I also have the same problem when creating a new Java class. This is quite
annoying. Thanks for looking into it and providing a patch. The
patched plugin works well for me.

Regarding the next actions, is it possible to backport this fix to
google-java-format 1.7.0 series, and release a new version for that?  The
latest version of 1.7.0.5 is released in Apr 2020. If this doesn't work,
option #2 also sounds good to me, because users need to download the plugin
anyway.

Best,
Jark



On Sun, 24 Jan 2021 at 03:32, Matthias Pohl  wrote:

> // With this one I am curious now how many of you had the same issue
> without complaining: In the engine team there were 4 out of 4.
>
> It's about the error dialog that pops up when creating a new Java class
> file. Additionally, the Java class is generated but is not formatted
> correctly. Reformatting the file helps. Confirming the dialog is annoying,
> though. I started looking into the issue and found an
> UnsupportedOperationException that led me to the actual cause: A bug in the
> google-java-format plugin version 1.7.0.5 is causing this behavior. I
> provided a more detailed description of my findings in FLINK-21106 [1]
> including a compiled version of the patched plugin.
>
> I want to open the discussion on how we want to deal with it. I see three
> options right now:
> 1. We leave it as it is right now as we consider this to be a minor thing.
> 2. We provide the patched google-java-format plugin as part of the docs.
> 3. We upgrade to Java 11 to be able to upgrade the google-java-format
> plugin as it was already mentioned earlier in the thread.
>
> None of the above options seem to be the right one to go for. Any thoughts
> on this?
>
> Best,
> Matthias
>
> [1] https://issues.apache.org/jira/browse/FLINK-21106
>
>
>
> On Wed, Dec 30, 2020 at 11:05 AM Chesnay Schepler 
> wrote:
>
>> 1) No, it is not possible to exclude certain code blocks from formatting.
>> There is a workaround though for this case; you can add en empty comment
>> (//) to the end of a line to prevent subsequent lines from being added
>> to it.
>> https://github.com/google/google-java-format/issues/137
>>
>> Note that any spotless-specific features would like not help us anyway,
>> unless we'd be fine with not using the IntelliJ plugin.
>>
>> 2) The code style has not been updated yet.
>> The indent choices with the google-java-format is either 2 spaces for
>> everything, or 4 spaces + 8 spaces for continuations.
>> In other words, 4 spaces is simply not an option.
>>
>> On 12/30/2020 9:09 AM, Jark Wu wrote:
>> > Hi,
>> >
>> > I have played with the format plugin these days and found some problems.
>> > Maybe some of them are personal taste.
>> >
>> > 1) Is it possible to disable auto-format for some code blocks?
>> > For example, the format of code generation
>> > StructuredObjectConverter#generateCode is manually
>> >   adjusted for readability. However, the format plugin breaks it and
>> it's
>> > hard to read now.
>> > See before[1] and after[2]. cc @Timo Walther 
>> >
>> > 2) Using 4 spaces or 8 spaces for continuation indent?
>> > AOSP uses 8 spaces for continuation indent.
>> > However, Flink Code Style suggests "Each new line should have one extra
>> > indentation relative to
>> > the line of the called entity" which means 4 spaces.
>> > Personally, I think 4 spaces may be more friendly for Java lambdas.  An
>> > example:
>> >
>> > 8 spaces:
>> >
>> > wrapClassLoader(
>> >  () ->
>> >  environment
>> >  .getModules()
>> >  .forEach(
>> >  (name, entry) ->
>> >  modules.put(
>> >  name,
>> >  createModule(
>> >  entry.asMap(),
>> > classLoader;
>> >
>> > 4 spaces:
>> >
>> > wrapClassLoader(
>> >  () ->
>> >  environment
>> >  .getModules()
>> >  .forEach(
>> >  (name, entry) ->
>> >  modules.put(name, createModule(entry.asMap(),
>> > classLoader;
>> >
>> >
>> >
>> > Best,
>> > Jark
>> >
>> > [1]:
>> >
>> https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L169
>> > [2]:
>> >
>> https://github.com/apache/flink/blob/master/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L170
>> > [3]:
>> >
>> https://flink.apache.org/contributing/code-style-and-quality-formatting.html#breaking-the-lines-of-too-long-statements
>> >
>> > On Tue, 29 Dec 2020 at 19:19, Chesnay Schepler 
>> wrote:
>> >
>> >> I have filed FLINK-20803
>> >>  for the issue that
>> 

Re: [ANNOUNCE] New formatting rules are now in effect

[Matthias Pohl]

Maybe, it's worth a try: I created issue #560 [1] for this. Let's see what
happens.

[1] https://github.com/google/google-java-format/issues/560

On Sun, Jan 24, 2021 at 4:17 AM Jark Wu  wrote:

> Hi Matthias,
>
> I also have the same problem when creating a new Java class. This is quite
> annoying. Thanks for looking into it and providing a patch. The
> patched plugin works well for me.
>
> Regarding the next actions, is it possible to backport this fix to
> google-java-format 1.7.0 series, and release a new version for that?  The
> latest version of 1.7.0.5 is released in Apr 2020. If this doesn't work,
> option #2 also sounds good to me, because users need to download the plugin
> anyway.
>
> Best,
> Jark
>
>
>
> On Sun, 24 Jan 2021 at 03:32, Matthias Pohl 
> wrote:
>
>> // With this one I am curious now how many of you had the same issue
>> without complaining: In the engine team there were 4 out of 4.
>>
>> It's about the error dialog that pops up when creating a new Java class
>> file. Additionally, the Java class is generated but is not formatted
>> correctly. Reformatting the file helps. Confirming the dialog is annoying,
>> though. I started looking into the issue and found an
>> UnsupportedOperationException that led me to the actual cause: A bug in the
>> google-java-format plugin version 1.7.0.5 is causing this behavior. I
>> provided a more detailed description of my findings in FLINK-21106 [1]
>> including a compiled version of the patched plugin.
>>
>> I want to open the discussion on how we want to deal with it. I see three
>> options right now:
>> 1. We leave it as it is right now as we consider this to be a minor thing.
>> 2. We provide the patched google-java-format plugin as part of the docs.
>> 3. We upgrade to Java 11 to be able to upgrade the google-java-format
>> plugin as it was already mentioned earlier in the thread.
>>
>> None of the above options seem to be the right one to go for. Any
>> thoughts on this?
>>
>> Best,
>> Matthias
>>
>> [1] https://issues.apache.org/jira/browse/FLINK-21106
>>
>>
>>
>> On Wed, Dec 30, 2020 at 11:05 AM Chesnay Schepler 
>> wrote:
>>
>>> 1) No, it is not possible to exclude certain code blocks from formatting.
>>> There is a workaround though for this case; you can add en empty comment
>>> (//) to the end of a line to prevent subsequent lines from being added
>>> to it.
>>> https://github.com/google/google-java-format/issues/137
>>>
>>> Note that any spotless-specific features would like not help us anyway,
>>> unless we'd be fine with not using the IntelliJ plugin.
>>>
>>> 2) The code style has not been updated yet.
>>> The indent choices with the google-java-format is either 2 spaces for
>>> everything, or 4 spaces + 8 spaces for continuations.
>>> In other words, 4 spaces is simply not an option.
>>>
>>> On 12/30/2020 9:09 AM, Jark Wu wrote:
>>> > Hi,
>>> >
>>> > I have played with the format plugin these days and found some
>>> problems.
>>> > Maybe some of them are personal taste.
>>> >
>>> > 1) Is it possible to disable auto-format for some code blocks?
>>> > For example, the format of code generation
>>> > StructuredObjectConverter#generateCode is manually
>>> >   adjusted for readability. However, the format plugin breaks it and
>>> it's
>>> > hard to read now.
>>> > See before[1] and after[2]. cc @Timo Walther 
>>> >
>>> > 2) Using 4 spaces or 8 spaces for continuation indent?
>>> > AOSP uses 8 spaces for continuation indent.
>>> > However, Flink Code Style suggests "Each new line should have one extra
>>> > indentation relative to
>>> > the line of the called entity" which means 4 spaces.
>>> > Personally, I think 4 spaces may be more friendly for Java lambdas.  An
>>> > example:
>>> >
>>> > 8 spaces:
>>> >
>>> > wrapClassLoader(
>>> >  () ->
>>> >  environment
>>> >  .getModules()
>>> >  .forEach(
>>> >  (name, entry) ->
>>> >  modules.put(
>>> >  name,
>>> >  createModule(
>>> >  entry.asMap(),
>>> > classLoader;
>>> >
>>> > 4 spaces:
>>> >
>>> > wrapClassLoader(
>>> >  () ->
>>> >  environment
>>> >  .getModules()
>>> >  .forEach(
>>> >  (name, entry) ->
>>> >  modules.put(name, createModule(entry.asMap(),
>>> > classLoader;
>>> >
>>> >
>>> >
>>> > Best,
>>> > Jark
>>> >
>>> > [1]:
>>> >
>>> https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L169
>>> > [2]:
>>> >
>>> https://github.com/apache/flink/blob/master/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConve

Re: [ANNOUNCE] New formatting rules are now in effect

[Till Rohrmann]

Thanks for looking into this problem Matthias. The patched version also
works for me. +1 for what Jark proposed.

Cheers,
Till

On Sun, Jan 24, 2021 at 3:50 PM Matthias Pohl 
wrote:

> Maybe, it's worth a try: I created issue #560 [1] for this. Let's see what
> happens.
>
> [1] https://github.com/google/google-java-format/issues/560
>
> On Sun, Jan 24, 2021 at 4:17 AM Jark Wu  wrote:
>
>> Hi Matthias,
>>
>> I also have the same problem when creating a new Java class. This is
>> quite annoying. Thanks for looking into it and providing a patch. The
>> patched plugin works well for me.
>>
>> Regarding the next actions, is it possible to backport this fix to
>> google-java-format 1.7.0 series, and release a new version for that?  The
>> latest version of 1.7.0.5 is released in Apr 2020. If this doesn't work,
>> option #2 also sounds good to me, because users need to download the plugin
>> anyway.
>>
>> Best,
>> Jark
>>
>>
>>
>> On Sun, 24 Jan 2021 at 03:32, Matthias Pohl 
>> wrote:
>>
>>> // With this one I am curious now how many of you had the same issue
>>> without complaining: In the engine team there were 4 out of 4.
>>>
>>> It's about the error dialog that pops up when creating a new Java class
>>> file. Additionally, the Java class is generated but is not formatted
>>> correctly. Reformatting the file helps. Confirming the dialog is annoying,
>>> though. I started looking into the issue and found an
>>> UnsupportedOperationException that led me to the actual cause: A bug in the
>>> google-java-format plugin version 1.7.0.5 is causing this behavior. I
>>> provided a more detailed description of my findings in FLINK-21106 [1]
>>> including a compiled version of the patched plugin.
>>>
>>> I want to open the discussion on how we want to deal with it. I see
>>> three options right now:
>>> 1. We leave it as it is right now as we consider this to be a minor
>>> thing.
>>> 2. We provide the patched google-java-format plugin as part of the docs.
>>> 3. We upgrade to Java 11 to be able to upgrade the google-java-format
>>> plugin as it was already mentioned earlier in the thread.
>>>
>>> None of the above options seem to be the right one to go for. Any
>>> thoughts on this?
>>>
>>> Best,
>>> Matthias
>>>
>>> [1] https://issues.apache.org/jira/browse/FLINK-21106
>>>
>>>
>>>
>>> On Wed, Dec 30, 2020 at 11:05 AM Chesnay Schepler 
>>> wrote:
>>>
 1) No, it is not possible to exclude certain code blocks from
 formatting.
 There is a workaround though for this case; you can add en empty
 comment
 (//) to the end of a line to prevent subsequent lines from being added
 to it.
 https://github.com/google/google-java-format/issues/137

 Note that any spotless-specific features would like not help us anyway,
 unless we'd be fine with not using the IntelliJ plugin.

 2) The code style has not been updated yet.
 The indent choices with the google-java-format is either 2 spaces for
 everything, or 4 spaces + 8 spaces for continuations.
 In other words, 4 spaces is simply not an option.

 On 12/30/2020 9:09 AM, Jark Wu wrote:
 > Hi,
 >
 > I have played with the format plugin these days and found some
 problems.
 > Maybe some of them are personal taste.
 >
 > 1) Is it possible to disable auto-format for some code blocks?
 > For example, the format of code generation
 > StructuredObjectConverter#generateCode is manually
 >   adjusted for readability. However, the format plugin breaks it and
 it's
 > hard to read now.
 > See before[1] and after[2]. cc @Timo Walther 
 >
 > 2) Using 4 spaces or 8 spaces for continuation indent?
 > AOSP uses 8 spaces for continuation indent.
 > However, Flink Code Style suggests "Each new line should have one
 extra
 > indentation relative to
 > the line of the called entity" which means 4 spaces.
 > Personally, I think 4 spaces may be more friendly for Java lambdas.
 An
 > example:
 >
 > 8 spaces:
 >
 > wrapClassLoader(
 >  () ->
 >  environment
 >  .getModules()
 >  .forEach(
 >  (name, entry) ->
 >  modules.put(
 >  name,
 >  createModule(
 >
 entry.asMap(),
 > classLoader;
 >
 > 4 spaces:
 >
 > wrapClassLoader(
 >  () ->
 >  environment
 >  .getModules()
 >  .forEach(
 >  (name, entry) ->
 >  modules.put(name, createModule(entry.asMap(),
 > classLoader;
 >
 >
 >
 > Best,
 > Jark
 >
 > [1]:
 >
 https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink

Re: [ANNOUNCE] New formatting rules are now in effect

[Aljoscha Krettek]

I've always been using the most recent IntelliJ plugin version and it's 
fine for all of my code so far and it was never a problem when I worked 
on the initial reformatting.  For the rare case where more recent 
versions of the plugin would produce formatting that is incompatible 
with 1.7.5 our CI would catch it and I can run `mvn spotless:apply` to 
fix it.


I know that Chesnay added the explanation to pin the IntelliJ plugin to 
1.7.5 because of problems reported by Arvid (I believe), but I don't 
know in what cases newer plugin versions were producing incompatible 
formatting.


Maybe we can just use the most recent plugin in IntelliJ and it's fine 
99% of the time?


Best,
Aljoscha

Re: [ANNOUNCE] New formatting rules are now in effect

[Xintong Song]

Great! Thanks Aljoscha and Chesnay for driving this.

Thank you~

Xintong Song



On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler  wrote:

> Hello everyone,
>
> I have just merged the commits for FLINK-20651
>  to master,
> release-1.12 and release-11, which enforces new formatting rules using
> the spotless plugin, following the google-java-format.
>
> This change touched every single java file in the repository,
> predominantly due to switching from tabs to spaces. This implies that
> every PR and WIP branch will require a rebase.
>
>
> Most of the changes were done by a single commit, which you can exclude
> from git blame by configuring git as follows (note that this requires
> git 2.23+, and also works for IntelliJ):
>
> git config blame.ignoreRevsFile .git-blame-ignore-revs
>
>
> You can setup the IDE to follow the new code style as follows:
>
> 1. Install the google-java-format plugin
>  and
> enable it for the project
> 2. In the plugin settings, change the code style to "AOSP" (4-space
> indents)
> 3. Install the Save Actions plugin
> 
> 4. Enable the plugin, along with "Optimize imports" and "Reformat file"
>
> To manually apply the formatting, run:
>
> mvn com.diffplug.spotless:spotless-maven-plugin:apply
>
>
> Please be on the lookout for any suspicious formatting, outdated
> instructions or other inconveniences.
>
>
> Finally, a big thank you to Aljoscha for pushing this topic and finally
> bringing it to an end.
>
>

Re: [ANNOUNCE] New formatting rules are now in effect

[Jark Wu]

Thanks Aljoscha and Chesnay for the great work!

Best,
Jark

On Tue, 29 Dec 2020 at 11:11, Xintong Song  wrote:

> Great! Thanks Aljoscha and Chesnay for driving this.
>
> Thank you~
>
> Xintong Song
>
>
>
> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler 
> wrote:
>
> > Hello everyone,
> >
> > I have just merged the commits for FLINK-20651
> >  to master,
> > release-1.12 and release-11, which enforces new formatting rules using
> > the spotless plugin, following the google-java-format.
> >
> > This change touched every single java file in the repository,
> > predominantly due to switching from tabs to spaces. This implies that
> > every PR and WIP branch will require a rebase.
> >
> >
> > Most of the changes were done by a single commit, which you can exclude
> > from git blame by configuring git as follows (note that this requires
> > git 2.23+, and also works for IntelliJ):
> >
> > git config blame.ignoreRevsFile .git-blame-ignore-revs
> >
> >
> > You can setup the IDE to follow the new code style as follows:
> >
> > 1. Install the google-java-format plugin
> >  and
> > enable it for the project
> > 2. In the plugin settings, change the code style to "AOSP" (4-space
> > indents)
> > 3. Install the Save Actions plugin
> > 
> > 4. Enable the plugin, along with "Optimize imports" and "Reformat file"
> >
> > To manually apply the formatting, run:
> >
> > mvn com.diffplug.spotless:spotless-maven-plugin:apply
> >
> >
> > Please be on the lookout for any suspicious formatting, outdated
> > instructions or other inconveniences.
> >
> >
> > Finally, a big thank you to Aljoscha for pushing this topic and finally
> > bringing it to an end.
> >
> >
>

Re: [ANNOUNCE] New formatting rules are now in effect

[Matthias Pohl]

Yes, thanks for driving this, Aljoscha. ...and Chesnay as well for helping
to finalize it.
Good job.

Matthias

On Tue, Dec 29, 2020 at 5:23 AM Jark Wu  wrote:

> Thanks Aljoscha and Chesnay for the great work!
>
> Best,
> Jark
>
> On Tue, 29 Dec 2020 at 11:11, Xintong Song  wrote:
>
> > Great! Thanks Aljoscha and Chesnay for driving this.
> >
> > Thank you~
> >
> > Xintong Song
> >
> >
> >
> > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler 
> > wrote:
> >
> > > Hello everyone,
> > >
> > > I have just merged the commits for FLINK-20651
> > >  to master,
> > > release-1.12 and release-11, which enforces new formatting rules using
> > > the spotless plugin, following the google-java-format.
> > >
> > > This change touched every single java file in the repository,
> > > predominantly due to switching from tabs to spaces. This implies that
> > > every PR and WIP branch will require a rebase.
> > >
> > >
> > > Most of the changes were done by a single commit, which you can exclude
> > > from git blame by configuring git as follows (note that this requires
> > > git 2.23+, and also works for IntelliJ):
> > >
> > > git config blame.ignoreRevsFile .git-blame-ignore-revs
> > >
> > >
> > > You can setup the IDE to follow the new code style as follows:
> > >
> > > 1. Install the google-java-format plugin
> > >  and
> > > enable it for the project
> > > 2. In the plugin settings, change the code style to "AOSP" (4-space
> > > indents)
> > > 3. Install the Save Actions plugin
> > > 
> > > 4. Enable the plugin, along with "Optimize imports" and "Reformat file"
> > >
> > > To manually apply the formatting, run:
> > >
> > > mvn com.diffplug.spotless:spotless-maven-plugin:apply
> > >
> > >
> > > Please be on the lookout for any suspicious formatting, outdated
> > > instructions or other inconveniences.
> > >
> > >
> > > Finally, a big thank you to Aljoscha for pushing this topic and finally
> > > bringing it to an end.
> > >
> > >
> >

Re: [ANNOUNCE] New formatting rules are now in effect

[Till Rohrmann]

Thanks a lot for this effort Aljoscha and Chesnay! Finally we have a common
code style :-)

Cheers,
Till

On Tue, Dec 29, 2020 at 7:32 AM Matthias Pohl 
wrote:

> Yes, thanks for driving this, Aljoscha. ...and Chesnay as well for helping
> to finalize it.
> Good job.
>
> Matthias
>
> On Tue, Dec 29, 2020 at 5:23 AM Jark Wu  wrote:
>
> > Thanks Aljoscha and Chesnay for the great work!
> >
> > Best,
> > Jark
> >
> > On Tue, 29 Dec 2020 at 11:11, Xintong Song 
> wrote:
> >
> > > Great! Thanks Aljoscha and Chesnay for driving this.
> > >
> > > Thank you~
> > >
> > > Xintong Song
> > >
> > >
> > >
> > > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler 
> > > wrote:
> > >
> > > > Hello everyone,
> > > >
> > > > I have just merged the commits for FLINK-20651
> > > >  to master,
> > > > release-1.12 and release-11, which enforces new formatting rules
> using
> > > > the spotless plugin, following the google-java-format.
> > > >
> > > > This change touched every single java file in the repository,
> > > > predominantly due to switching from tabs to spaces. This implies that
> > > > every PR and WIP branch will require a rebase.
> > > >
> > > >
> > > > Most of the changes were done by a single commit, which you can
> exclude
> > > > from git blame by configuring git as follows (note that this requires
> > > > git 2.23+, and also works for IntelliJ):
> > > >
> > > > git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > >
> > > >
> > > > You can setup the IDE to follow the new code style as follows:
> > > >
> > > > 1. Install the google-java-format plugin
> > > >  and
> > > > enable it for the project
> > > > 2. In the plugin settings, change the code style to "AOSP" (4-space
> > > > indents)
> > > > 3. Install the Save Actions plugin
> > > > 
> > > > 4. Enable the plugin, along with "Optimize imports" and "Reformat
> file"
> > > >
> > > > To manually apply the formatting, run:
> > > >
> > > > mvn com.diffplug.spotless:spotless-maven-plugin:apply
> > > >
> > > >
> > > > Please be on the lookout for any suspicious formatting, outdated
> > > > instructions or other inconveniences.
> > > >
> > > >
> > > > Finally, a big thank you to Aljoscha for pushing this topic and
> finally
> > > > bringing it to an end.
> > > >
> > > >
> > >
>

Re: [ANNOUNCE] New formatting rules are now in effect

[Flavio Pompermaier]

Thanks Aljoscha and Chesnay for this small but important improvement!
In the new year writing new Flink features will be funnier than ever ;)

On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann  wrote:

> Thanks a lot for this effort Aljoscha and Chesnay! Finally we have a common
> code style :-)
>
> Cheers,
> Till
>
> On Tue, Dec 29, 2020 at 7:32 AM Matthias Pohl 
> wrote:
>
> > Yes, thanks for driving this, Aljoscha. ...and Chesnay as well for
> helping
> > to finalize it.
> > Good job.
> >
> > Matthias
> >
> > On Tue, Dec 29, 2020 at 5:23 AM Jark Wu  wrote:
> >
> > > Thanks Aljoscha and Chesnay for the great work!
> > >
> > > Best,
> > > Jark
> > >
> > > On Tue, 29 Dec 2020 at 11:11, Xintong Song 
> > wrote:
> > >
> > > > Great! Thanks Aljoscha and Chesnay for driving this.
> > > >
> > > > Thank you~
> > > >
> > > > Xintong Song
> > > >
> > > >
> > > >
> > > > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler  >
> > > > wrote:
> > > >
> > > > > Hello everyone,
> > > > >
> > > > > I have just merged the commits for FLINK-20651
> > > > >  to master,
> > > > > release-1.12 and release-11, which enforces new formatting rules
> > using
> > > > > the spotless plugin, following the google-java-format.
> > > > >
> > > > > This change touched every single java file in the repository,
> > > > > predominantly due to switching from tabs to spaces. This implies
> that
> > > > > every PR and WIP branch will require a rebase.
> > > > >
> > > > >
> > > > > Most of the changes were done by a single commit, which you can
> > exclude
> > > > > from git blame by configuring git as follows (note that this
> requires
> > > > > git 2.23+, and also works for IntelliJ):
> > > > >
> > > > > git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > > >
> > > > >
> > > > > You can setup the IDE to follow the new code style as follows:
> > > > >
> > > > > 1. Install the google-java-format plugin
> > > > >  and
> > > > > enable it for the project
> > > > > 2. In the plugin settings, change the code style to "AOSP" (4-space
> > > > > indents)
> > > > > 3. Install the Save Actions plugin
> > > > > 
> > > > > 4. Enable the plugin, along with "Optimize imports" and "Reformat
> > file"
> > > > >
> > > > > To manually apply the formatting, run:
> > > > >
> > > > > mvn com.diffplug.spotless:spotless-maven-plugin:apply
> > > > >
> > > > >
> > > > > Please be on the lookout for any suspicious formatting, outdated
> > > > > instructions or other inconveniences.
> > > > >
> > > > >
> > > > > Finally, a big thank you to Aljoscha for pushing this topic and
> > finally
> > > > > bringing it to an end.
> > > > >
> > > > >
> > > >
> >

Re: [ANNOUNCE] New formatting rules are now in effect

[Till Rohrmann]

I might have found a problem:

In my current setup I am using the google-java-format plugin version
1.9.0.0 which uses google-java-format 1.9.0. In our spotless configuration
we are using google-java-format 1.7.0, however. The result is that spotless
formats my code differently than IntelliJ. The following snippet shows the
difference:

IntelliJ formatting with google-java-format 1.9.0:

return
Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))
   .map(
   accessExecutionJobVertex ->

 Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
   .orElse(Collections.emptyList())
   .stream()
   .map(AccessExecutionVertex::getCurrentExecutionAttempt)
   .collect(Collectors.toList());

Spotless formatting with google-java-format 1.7.0:

return
Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))
  .map(
  accessExecutionJobVertex ->

Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
  .orElse(Collections.emptyList()).stream()
  .map(AccessExecutionVertex::getCurrentExecutionAttempt)
  .collect(Collectors.toList());

Note that the .stream() method is in the same line as .orElse().

I think this raises a bit the question which versions do we want to use?
Manually installing google-java-format plugin version 1.7.0.5 solved the
problem for me.

Cheers,
Till

On Tue, Dec 29, 2020 at 11:58 AM Flavio Pompermaier 
wrote:

> Thanks Aljoscha and Chesnay for this small but important improvement!
> In the new year writing new Flink features will be funnier than ever ;)
>
> On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann 
> wrote:
>
> > Thanks a lot for this effort Aljoscha and Chesnay! Finally we have a
> common
> > code style :-)
> >
> > Cheers,
> > Till
> >
> > On Tue, Dec 29, 2020 at 7:32 AM Matthias Pohl 
> > wrote:
> >
> > > Yes, thanks for driving this, Aljoscha. ...and Chesnay as well for
> > helping
> > > to finalize it.
> > > Good job.
> > >
> > > Matthias
> > >
> > > On Tue, Dec 29, 2020 at 5:23 AM Jark Wu  wrote:
> > >
> > > > Thanks Aljoscha and Chesnay for the great work!
> > > >
> > > > Best,
> > > > Jark
> > > >
> > > > On Tue, 29 Dec 2020 at 11:11, Xintong Song 
> > > wrote:
> > > >
> > > > > Great! Thanks Aljoscha and Chesnay for driving this.
> > > > >
> > > > > Thank you~
> > > > >
> > > > > Xintong Song
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
> ches...@apache.org
> > >
> > > > > wrote:
> > > > >
> > > > > > Hello everyone,
> > > > > >
> > > > > > I have just merged the commits for FLINK-20651
> > > > > >  to master,
> > > > > > release-1.12 and release-11, which enforces new formatting rules
> > > using
> > > > > > the spotless plugin, following the google-java-format.
> > > > > >
> > > > > > This change touched every single java file in the repository,
> > > > > > predominantly due to switching from tabs to spaces. This implies
> > that
> > > > > > every PR and WIP branch will require a rebase.
> > > > > >
> > > > > >
> > > > > > Most of the changes were done by a single commit, which you can
> > > exclude
> > > > > > from git blame by configuring git as follows (note that this
> > requires
> > > > > > git 2.23+, and also works for IntelliJ):
> > > > > >
> > > > > > git config blame.ignoreRevsFile .git-blame-ignore-revs
> > > > > >
> > > > > >
> > > > > > You can setup the IDE to follow the new code style as follows:
> > > > > >
> > > > > > 1. Install the google-java-format plugin
> > > > > > 
> and
> > > > > > enable it for the project
> > > > > > 2. In the plugin settings, change the code style to "AOSP"
> (4-space
> > > > > > indents)
> > > > > > 3. Install the Save Actions plugin
> > > > > > 
> > > > > > 4. Enable the plugin, along with "Optimize imports" and "Reformat
> > > file"
> > > > > >
> > > > > > To manually apply the formatting, run:
> > > > > >
> > > > > > mvn com.diffplug.spotless:spotless-maven-plugin:apply
> > > > > >
> > > > > >
> > > > > > Please be on the lookout for any suspicious formatting, outdated
> > > > > > instructions or other inconveniences.
> > > > > >
> > > > > >
> > > > > > Finally, a big thank you to Aljoscha for pushing this topic and
> > > finally
> > > > > > bringing it to an end.
> > > > > >
> > > > > >
> > > > >
> > >
>

Re: [ANNOUNCE] New formatting rules are now in effect

[Chesnay Schepler]

I have filed FLINK-20803 
 for the issue that 
Till raised.


google-java-format 1.8 and above require Java 11 to /run/, so we'll 
stick with 1.7 for the time being.
This requires a manual installation of the google-java-format plugin, 
and remembering to not update the plugin (urgh...).


Long term we may want to think about requiring Java 11 for /development/ 
(*not* usage) of Flink.


On 12/29/2020 12:13 PM, Till Rohrmann wrote:

I might have found a problem:

In my current setup I am using the google-java-format plugin version
1.9.0.0 which uses google-java-format 1.9.0. In our spotless configuration
we are using google-java-format 1.7.0, however. The result is that spotless
formats my code differently than IntelliJ. The following snippet shows the
difference:

IntelliJ formatting with google-java-format 1.9.0:

return
Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))
.map(
accessExecutionJobVertex ->

  Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
.orElse(Collections.emptyList())
.stream()
.map(AccessExecutionVertex::getCurrentExecutionAttempt)
.collect(Collectors.toList());

Spotless formatting with google-java-format 1.7.0:

return
Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))
   .map(
   accessExecutionJobVertex ->

Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
   .orElse(Collections.emptyList()).stream()
   .map(AccessExecutionVertex::getCurrentExecutionAttempt)
   .collect(Collectors.toList());

Note that the .stream() method is in the same line as .orElse().

I think this raises a bit the question which versions do we want to use?
Manually installing google-java-format plugin version 1.7.0.5 solved the
problem for me.

Cheers,
Till

On Tue, Dec 29, 2020 at 11:58 AM Flavio Pompermaier 
wrote:


Thanks Aljoscha and Chesnay for this small but important improvement!
In the new year writing new Flink features will be funnier than ever ;)

On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann 
wrote:


Thanks a lot for this effort Aljoscha and Chesnay! Finally we have a

common

code style :-)

Cheers,
Till

On Tue, Dec 29, 2020 at 7:32 AM Matthias Pohl 
wrote:


Yes, thanks for driving this, Aljoscha. ...and Chesnay as well for

helping

to finalize it.
Good job.

Matthias

On Tue, Dec 29, 2020 at 5:23 AM Jark Wu  wrote:


Thanks Aljoscha and Chesnay for the great work!

Best,
Jark

On Tue, 29 Dec 2020 at 11:11, Xintong Song 

wrote:

Great! Thanks Aljoscha and Chesnay for driving this.

Thank you~

Xintong Song



On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <

ches...@apache.org

wrote:


Hello everyone,

I have just merged the commits for FLINK-20651
 to master,
release-1.12 and release-11, which enforces new formatting rules

using

the spotless plugin, following the google-java-format.

This change touched every single java file in the repository,
predominantly due to switching from tabs to spaces. This implies

that

every PR and WIP branch will require a rebase.


Most of the changes were done by a single commit, which you can

exclude

from git blame by configuring git as follows (note that this

requires

git 2.23+, and also works for IntelliJ):

git config blame.ignoreRevsFile .git-blame-ignore-revs


You can setup the IDE to follow the new code style as follows:

1. Install the google-java-format plugin


and

enable it for the project
2. In the plugin settings, change the code style to "AOSP"

(4-space

indents)
3. Install the Save Actions plugin

4. Enable the plugin, along with "Optimize imports" and "Reformat

file"

To manually apply the formatting, run:

mvn com.diffplug.spotless:spotless-maven-plugin:apply


Please be on the lookout for any suspicious formatting, outdated
instructions or other inconveniences.


Finally, a big thank you to Aljoscha for pushing this topic and

finally

bringing it to an end.

Re: [ANNOUNCE] New formatting rules are now in effect

[Jark Wu]

Hi,

I have played with the format plugin these days and found some problems.
Maybe some of them are personal taste.

1) Is it possible to disable auto-format for some code blocks?
For example, the format of code generation
StructuredObjectConverter#generateCode is manually
 adjusted for readability. However, the format plugin breaks it and it's
hard to read now.
See before[1] and after[2]. cc @Timo Walther 

2) Using 4 spaces or 8 spaces for continuation indent?
AOSP uses 8 spaces for continuation indent.
However, Flink Code Style suggests "Each new line should have one extra
indentation relative to
the line of the called entity" which means 4 spaces.
Personally, I think 4 spaces may be more friendly for Java lambdas.  An
example:

8 spaces:

wrapClassLoader(
() ->
environment
.getModules()
.forEach(
(name, entry) ->
modules.put(
name,
createModule(
entry.asMap(),
classLoader;

4 spaces:

wrapClassLoader(
() ->
environment
.getModules()
.forEach(
(name, entry) ->
modules.put(name, createModule(entry.asMap(),
classLoader;



Best,
Jark

[1]:
https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L169
[2]:
https://github.com/apache/flink/blob/master/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L170
[3]:
https://flink.apache.org/contributing/code-style-and-quality-formatting.html#breaking-the-lines-of-too-long-statements

On Tue, 29 Dec 2020 at 19:19, Chesnay Schepler  wrote:

> I have filed FLINK-20803
>  for the issue that
> Till raised.
>
> google-java-format 1.8 and above require Java 11 to /run/, so we'll
> stick with 1.7 for the time being.
> This requires a manual installation of the google-java-format plugin,
> and remembering to not update the plugin (urgh...).
>
> Long term we may want to think about requiring Java 11 for /development/
> (*not* usage) of Flink.
>
> On 12/29/2020 12:13 PM, Till Rohrmann wrote:
> > I might have found a problem:
> >
> > In my current setup I am using the google-java-format plugin version
> > 1.9.0.0 which uses google-java-format 1.9.0. In our spotless
> configuration
> > we are using google-java-format 1.7.0, however. The result is that
> spotless
> > formats my code differently than IntelliJ. The following snippet shows
> the
> > difference:
> >
> > IntelliJ formatting with google-java-format 1.9.0:
> >
> > return
> >
> Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))
> > .map(
> > accessExecutionJobVertex ->
> >
> >   Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
> > .orElse(Collections.emptyList())
> > .stream()
> > .map(AccessExecutionVertex::getCurrentExecutionAttempt)
> > .collect(Collectors.toList());
> >
> > Spotless formatting with google-java-format 1.7.0:
> >
> > return
> >
> Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))
> >.map(
> >accessExecutionJobVertex ->
> >
> > Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
> >.orElse(Collections.emptyList()).stream()
> >.map(AccessExecutionVertex::getCurrentExecutionAttempt)
> >.collect(Collectors.toList());
> >
> > Note that the .stream() method is in the same line as .orElse().
> >
> > I think this raises a bit the question which versions do we want to use?
> > Manually installing google-java-format plugin version 1.7.0.5 solved the
> > problem for me.
> >
> > Cheers,
> > Till
> >
> > On Tue, Dec 29, 2020 at 11:58 AM Flavio Pompermaier <
> pomperma...@okkam.it>
> > wrote:
> >
> >> Thanks Aljoscha and Chesnay for this small but important improvement!
> >> In the new year writing new Flink features will be funnier than ever ;)
> >>
> >> On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann 
> >> wrote:
> >>
> >>> Thanks a lot for this effort Aljoscha and Chesnay! Finally we have a
> >> common
> >>> code style :-)
> >>>
> >>> Cheers,
> >>> Till
> >>>
> >>> On Tue, Dec 29, 2020 at 7:32 AM Matthias Pohl 
> >>> wrote:
> >>>
>  Yes, thanks for driving this, Aljoscha. ...and Chesnay as well for
> >>> helping
>  to finalize it.
>  Good job.
> 
>  Matthias
> 
>  On Tue, Dec 29, 2020 at 5:23 AM Jark Wu  wrote:
> 
> > Thanks Aljoscha and Chesnay for the great work!
> >
> > Best,
> > Jark
> >
> > On Tue, 29 Dec 2020 at 11:11, Xintong Song 
>  wrote:
> >

Re: [ANNOUNCE] New formatting rules are now in effect

[Chesnay Schepler]

1) No, it is not possible to exclude certain code blocks from formatting.
There is a workaround though for this case; you can add en empty comment 
(//) to the end of a line to prevent subsequent lines from being added 
to it.

https://github.com/google/google-java-format/issues/137

Note that any spotless-specific features would like not help us anyway, 
unless we'd be fine with not using the IntelliJ plugin.


2) The code style has not been updated yet.
The indent choices with the google-java-format is either 2 spaces for 
everything, or 4 spaces + 8 spaces for continuations.

In other words, 4 spaces is simply not an option.

On 12/30/2020 9:09 AM, Jark Wu wrote:

Hi,

I have played with the format plugin these days and found some problems.
Maybe some of them are personal taste.

1) Is it possible to disable auto-format for some code blocks?
For example, the format of code generation
StructuredObjectConverter#generateCode is manually
  adjusted for readability. However, the format plugin breaks it and it's
hard to read now.
See before[1] and after[2]. cc @Timo Walther 

2) Using 4 spaces or 8 spaces for continuation indent?
AOSP uses 8 spaces for continuation indent.
However, Flink Code Style suggests "Each new line should have one extra
indentation relative to
the line of the called entity" which means 4 spaces.
Personally, I think 4 spaces may be more friendly for Java lambdas.  An
example:

8 spaces:

wrapClassLoader(
 () ->
 environment
 .getModules()
 .forEach(
 (name, entry) ->
 modules.put(
 name,
 createModule(
 entry.asMap(),
classLoader;

4 spaces:

wrapClassLoader(
 () ->
 environment
 .getModules()
 .forEach(
 (name, entry) ->
 modules.put(name, createModule(entry.asMap(),
classLoader;



Best,
Jark

[1]:
https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L169
[2]:
https://github.com/apache/flink/blob/master/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L170
[3]:
https://flink.apache.org/contributing/code-style-and-quality-formatting.html#breaking-the-lines-of-too-long-statements

On Tue, 29 Dec 2020 at 19:19, Chesnay Schepler  wrote:


I have filed FLINK-20803
 for the issue that
Till raised.

google-java-format 1.8 and above require Java 11 to /run/, so we'll
stick with 1.7 for the time being.
This requires a manual installation of the google-java-format plugin,
and remembering to not update the plugin (urgh...).

Long term we may want to think about requiring Java 11 for /development/
(*not* usage) of Flink.

On 12/29/2020 12:13 PM, Till Rohrmann wrote:

I might have found a problem:

In my current setup I am using the google-java-format plugin version
1.9.0.0 which uses google-java-format 1.9.0. In our spotless

configuration

we are using google-java-format 1.7.0, however. The result is that

spotless

formats my code differently than IntelliJ. The following snippet shows

the

difference:

IntelliJ formatting with google-java-format 1.9.0:

return


Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))

 .map(
 accessExecutionJobVertex ->

   Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
 .orElse(Collections.emptyList())
 .stream()
 .map(AccessExecutionVertex::getCurrentExecutionAttempt)
 .collect(Collectors.toList());

Spotless formatting with google-java-format 1.7.0:

return


Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId))

.map(
accessExecutionJobVertex ->

Arrays.asList(accessExecutionJobVertex.getTaskVertices()))
.orElse(Collections.emptyList()).stream()
.map(AccessExecutionVertex::getCurrentExecutionAttempt)
.collect(Collectors.toList());

Note that the .stream() method is in the same line as .orElse().

I think this raises a bit the question which versions do we want to use?
Manually installing google-java-format plugin version 1.7.0.5 solved the
problem for me.

Cheers,
Till

On Tue, Dec 29, 2020 at 11:58 AM Flavio Pompermaier <

pomperma...@okkam.it>

wrote:


Thanks Aljoscha and Chesnay for this small but important improvement!
In the new year writing new Flink features will be funnier than ever ;)

On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann 
wrote:


Thanks a lot for this effort Aljoscha and Chesnay! Finally we have a

common

code style :-)

Cheers,
Till

On Tu