[ANNOUNCE] New formatting rules are now in effect

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[ANNOUNCE] New formatting rules are now in effect

Chesnay Schepler-3
Hello everyone,

I have just merged the commits for FLINK-20651
<https://issues.apache.org/jira/browse/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
<https://plugins.jetbrains.com/plugin/8527-google-java-format> 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
<https://plugins.jetbrains.com/plugin/7642-save-actions>
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.

Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> wrote:

> Hello everyone,
>
> I have just merged the commits for FLINK-20651
> <https://issues.apache.org/jira/browse/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
> <https://plugins.jetbrains.com/plugin/8527-google-java-format> 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
> <https://plugins.jetbrains.com/plugin/7642-save-actions>
> 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.
>
>
Reply | Threaded
Open this post in threaded view
|

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

Jark Wu-2
Thanks Aljoscha and Chesnay for the great work!

Best,
Jark

On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]> wrote:

> Great! Thanks Aljoscha and Chesnay for driving this.
>
> Thank you~
>
> Xintong Song
>
>
>
> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <[hidden email]>
> wrote:
>
> > Hello everyone,
> >
> > I have just merged the commits for FLINK-20651
> > <https://issues.apache.org/jira/browse/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
> > <https://plugins.jetbrains.com/plugin/8527-google-java-format> 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
> > <https://plugins.jetbrains.com/plugin/7642-save-actions>
> > 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.
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

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

Matthias
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 <[hidden email]> wrote:

> Thanks Aljoscha and Chesnay for the great work!
>
> Best,
> Jark
>
> On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]> wrote:
>
> > Great! Thanks Aljoscha and Chesnay for driving this.
> >
> > Thank you~
> >
> > Xintong Song
> >
> >
> >
> > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <[hidden email]>
> > wrote:
> >
> > > Hello everyone,
> > >
> > > I have just merged the commits for FLINK-20651
> > > <https://issues.apache.org/jira/browse/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
> > > <https://plugins.jetbrains.com/plugin/8527-google-java-format> 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
> > > <https://plugins.jetbrains.com/plugin/7642-save-actions>
> > > 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.
> > >
> > >
> >
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]>
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 <[hidden email]> wrote:
>
> > Thanks Aljoscha and Chesnay for the great work!
> >
> > Best,
> > Jark
> >
> > On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
> wrote:
> >
> > > Great! Thanks Aljoscha and Chesnay for driving this.
> > >
> > > Thank you~
> > >
> > > Xintong Song
> > >
> > >
> > >
> > > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <[hidden email]>
> > > wrote:
> > >
> > > > Hello everyone,
> > > >
> > > > I have just merged the commits for FLINK-20651
> > > > <https://issues.apache.org/jira/browse/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
> > > > <https://plugins.jetbrains.com/plugin/8527-google-java-format> 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
> > > > <https://plugins.jetbrains.com/plugin/7642-save-actions>
> > > > 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.
> > > >
> > > >
> > >
>
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]> wrote:
> >
> > > Thanks Aljoscha and Chesnay for the great work!
> > >
> > > Best,
> > > Jark
> > >
> > > On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
> > wrote:
> > >
> > > > Great! Thanks Aljoscha and Chesnay for driving this.
> > > >
> > > > Thank you~
> > > >
> > > > Xintong Song
> > > >
> > > >
> > > >
> > > > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <[hidden email]
> >
> > > > wrote:
> > > >
> > > > > Hello everyone,
> > > > >
> > > > > I have just merged the commits for FLINK-20651
> > > > > <https://issues.apache.org/jira/browse/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
> > > > > <https://plugins.jetbrains.com/plugin/8527-google-java-format> 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
> > > > > <https://plugins.jetbrains.com/plugin/7642-save-actions>
> > > > > 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.
> > > > >
> > > > >
> > > >
> >
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]>
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 <[hidden email]>
> 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 <[hidden email]>
> > 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 <[hidden email]> wrote:
> > >
> > > > Thanks Aljoscha and Chesnay for the great work!
> > > >
> > > > Best,
> > > > Jark
> > > >
> > > > On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
> > > wrote:
> > > >
> > > > > Great! Thanks Aljoscha and Chesnay for driving this.
> > > > >
> > > > > Thank you~
> > > > >
> > > > > Xintong Song
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
> [hidden email]
> > >
> > > > > wrote:
> > > > >
> > > > > > Hello everyone,
> > > > > >
> > > > > > I have just merged the commits for FLINK-20651
> > > > > > <https://issues.apache.org/jira/browse/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
> > > > > > <https://plugins.jetbrains.com/plugin/8527-google-java-format>
> 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
> > > > > > <https://plugins.jetbrains.com/plugin/7642-save-actions>
> > > > > > 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.
> > > > > >
> > > > > >
> > > > >
> > >
>
Reply | Threaded
Open this post in threaded view
|

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

Chesnay Schepler-3
I have filed FLINK-20803
<https://issues.apache.org/jira/browse/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 <[hidden email]>
> 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 <[hidden email]>
>> 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 <[hidden email]>
>>> 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 <[hidden email]> wrote:
>>>>
>>>>> Thanks Aljoscha and Chesnay for the great work!
>>>>>
>>>>> Best,
>>>>> Jark
>>>>>
>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
>>>> wrote:
>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
>>>>>>
>>>>>> Thank you~
>>>>>>
>>>>>> Xintong Song
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
>> [hidden email]
>>>>>> wrote:
>>>>>>
>>>>>>> Hello everyone,
>>>>>>>
>>>>>>> I have just merged the commits for FLINK-20651
>>>>>>> <https://issues.apache.org/jira/browse/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
>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format>
>> 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
>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
>>>>>>> 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.
>>>>>>>
>>>>>>>

Reply | Threaded
Open this post in threaded view
|

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

Jark Wu-2
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 <[hidden email]>

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 <[hidden email]> wrote:

> I have filed FLINK-20803
> <https://issues.apache.org/jira/browse/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 <
> [hidden email]>
> > 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 <[hidden email]>
> >> 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 <[hidden email]>
> >>> 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 <[hidden email]> wrote:
> >>>>
> >>>>> Thanks Aljoscha and Chesnay for the great work!
> >>>>>
> >>>>> Best,
> >>>>> Jark
> >>>>>
> >>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
> >>>> wrote:
> >>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
> >>>>>>
> >>>>>> Thank you~
> >>>>>>
> >>>>>> Xintong Song
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
> >> [hidden email]
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Hello everyone,
> >>>>>>>
> >>>>>>> I have just merged the commits for FLINK-20651
> >>>>>>> <https://issues.apache.org/jira/browse/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
> >>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format>
> >> 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
> >>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
> >>>>>>> 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.
> >>>>>>>
> >>>>>>>
>
>
Reply | Threaded
Open this post in threaded view
|

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

Chesnay Schepler-3
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 <[hidden email]>
>
> 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 <[hidden email]> wrote:
>
>> I have filed FLINK-20803
>> <https://issues.apache.org/jira/browse/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 <
>> [hidden email]>
>>> 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 <[hidden email]>
>>>> 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 <[hidden email]>
>>>>> 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 <[hidden email]> wrote:
>>>>>>
>>>>>>> Thanks Aljoscha and Chesnay for the great work!
>>>>>>>
>>>>>>> Best,
>>>>>>> Jark
>>>>>>>
>>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
>>>>>> wrote:
>>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
>>>>>>>>
>>>>>>>> Thank you~
>>>>>>>>
>>>>>>>> Xintong Song
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
>>>> [hidden email]
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hello everyone,
>>>>>>>>>
>>>>>>>>> I have just merged the commits for FLINK-20651
>>>>>>>>> <https://issues.apache.org/jira/browse/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
>>>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format>
>>>> 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
>>>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Matthias
// 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 <[hidden email]>
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 <[hidden email]>
> >
> > 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 <[hidden email]>
> wrote:
> >
> >> I have filed FLINK-20803
> >> <https://issues.apache.org/jira/browse/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 <
> >> [hidden email]>
> >>> 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 <[hidden email]>
> >>>> 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 <
> [hidden email]>
> >>>>> 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 <[hidden email]> wrote:
> >>>>>>
> >>>>>>> Thanks Aljoscha and Chesnay for the great work!
> >>>>>>>
> >>>>>>> Best,
> >>>>>>> Jark
> >>>>>>>
> >>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]>
> >>>>>> wrote:
> >>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
> >>>>>>>>
> >>>>>>>> Thank you~
> >>>>>>>>
> >>>>>>>> Xintong Song
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
> >>>> [hidden email]
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Hello everyone,
> >>>>>>>>>
> >>>>>>>>> I have just merged the commits for FLINK-20651
> >>>>>>>>> <https://issues.apache.org/jira/browse/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
> >>>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format>
> >>>> 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
> >>>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
> >>>>>>>>> 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.
> >>>>>>>>>
> >>>>>>>>>
> >>
>
Reply | Threaded
Open this post in threaded view
|

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

Jark Wu-2
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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
>> >
>> > 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 <[hidden email]>
>> wrote:
>> >
>> >> I have filed FLINK-20803
>> >> <https://issues.apache.org/jira/browse/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 <
>> >> [hidden email]>
>> >>> 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 <[hidden email]>
>> >>>> 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 <
>> [hidden email]>
>> >>>>> 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 <[hidden email]> wrote:
>> >>>>>>
>> >>>>>>> Thanks Aljoscha and Chesnay for the great work!
>> >>>>>>>
>> >>>>>>> Best,
>> >>>>>>> Jark
>> >>>>>>>
>> >>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <[hidden email]
>> >
>> >>>>>> wrote:
>> >>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
>> >>>>>>>>
>> >>>>>>>> Thank you~
>> >>>>>>>>
>> >>>>>>>> Xintong Song
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
>> >>>> [hidden email]
>> >>>>>>>> wrote:
>> >>>>>>>>
>> >>>>>>>>> Hello everyone,
>> >>>>>>>>>
>> >>>>>>>>> I have just merged the commits for FLINK-20651
>> >>>>>>>>> <https://issues.apache.org/jira/browse/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
>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format>
>> >>>> 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
>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
>> >>>>>>>>> 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.
>> >>>>>>>>>
>> >>>>>>>>>
>> >>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

Matthias
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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
>> 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 <[hidden email]>
>>> >
>>> > 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 <[hidden email]>
>>> wrote:
>>> >
>>> >> I have filed FLINK-20803
>>> >> <https://issues.apache.org/jira/browse/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 <
>>> >> [hidden email]>
>>> >>> 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 <[hidden email]
>>> >
>>> >>>> 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 <
>>> [hidden email]>
>>> >>>>> 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 <[hidden email]> wrote:
>>> >>>>>>
>>> >>>>>>> Thanks Aljoscha and Chesnay for the great work!
>>> >>>>>>>
>>> >>>>>>> Best,
>>> >>>>>>> Jark
>>> >>>>>>>
>>> >>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <
>>> [hidden email]>
>>> >>>>>> wrote:
>>> >>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
>>> >>>>>>>>
>>> >>>>>>>> Thank you~
>>> >>>>>>>>
>>> >>>>>>>> Xintong Song
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
>>> >>>> [hidden email]
>>> >>>>>>>> wrote:
>>> >>>>>>>>
>>> >>>>>>>>> Hello everyone,
>>> >>>>>>>>>
>>> >>>>>>>>> I have just merged the commits for FLINK-20651
>>> >>>>>>>>> <https://issues.apache.org/jira/browse/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
>>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format>
>>> >>>> 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
>>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
>>> >>>>>>>>> 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.
>>> >>>>>>>>>
>>> >>>>>>>>>
>>> >>
>>>
>>

--

Matthias Pohl | Engineer

Follow us @VervericaData Ververica <https://www.ververica.com/>

--

Join Flink Forward <https://flink-forward.org/> - The Apache Flink
Conference

Stream Processing | Event Driven | Real Time

--

Ververica GmbH | Invalidenstrasse 115, 10115 Berlin, Germany

--
Ververica GmbH
Registered at Amtsgericht Charlottenburg: HRB 158244 B
Managing Directors: Yip Park Tung Jason, Jinwei (Kevin) Zhang, Karl Anton
Wehner
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]>
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 <[hidden email]> 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 <[hidden email]>
>> 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 <[hidden email]>
>>> 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 <[hidden email]>
>>>> >
>>>> > 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 <[hidden email]>
>>>> wrote:
>>>> >
>>>> >> I have filed FLINK-20803
>>>> >> <https://issues.apache.org/jira/browse/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 <
>>>> >> [hidden email]>
>>>> >>> 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 <
>>>> [hidden email]>
>>>> >>>> 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 <
>>>> [hidden email]>
>>>> >>>>> 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 <[hidden email]>
>>>> wrote:
>>>> >>>>>>
>>>> >>>>>>> Thanks Aljoscha and Chesnay for the great work!
>>>> >>>>>>>
>>>> >>>>>>> Best,
>>>> >>>>>>> Jark
>>>> >>>>>>>
>>>> >>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <
>>>> [hidden email]>
>>>> >>>>>> wrote:
>>>> >>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
>>>> >>>>>>>>
>>>> >>>>>>>> Thank you~
>>>> >>>>>>>>
>>>> >>>>>>>> Xintong Song
>>>> >>>>>>>>
>>>> >>>>>>>>
>>>> >>>>>>>>
>>>> >>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
>>>> >>>> [hidden email]
>>>> >>>>>>>> wrote:
>>>> >>>>>>>>
>>>> >>>>>>>>> Hello everyone,
>>>> >>>>>>>>>
>>>> >>>>>>>>> I have just merged the commits for FLINK-20651
>>>> >>>>>>>>> <https://issues.apache.org/jira/browse/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
>>>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format
>>>> >
>>>> >>>> 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
>>>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions>
>>>> >>>>>>>>> 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.
>>>> >>>>>>>>>
>>>> >>>>>>>>>
>>>> >>
>>>>
>>>
>
> --
>
> Matthias Pohl | Engineer
>
> Follow us @VervericaData Ververica <https://www.ververica.com/>
>
> --
>
> Join Flink Forward <https://flink-forward.org/> - The Apache Flink
> Conference
>
> Stream Processing | Event Driven | Real Time
>
> --
>
> Ververica GmbH | Invalidenstrasse 115, 10115 Berlin, Germany
>
> --
> Ververica GmbH
> Registered at Amtsgericht Charlottenburg: HRB 158244 B
> Managing Directors: Yip Park Tung Jason, Jinwei (Kevin) Zhang, Karl Anton
> Wehner
>
Reply | Threaded
Open this post in threaded view
|

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

Aljoscha Krettek-2
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