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. |
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. > > |
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. > > > > > |
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. > > > > > > > > |
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. > > > > > > > > > > > > |
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. > > > > > > > > > > > > > > > > |
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. > > > > > > > > > > > > > > > > > > > > > |
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. >>>>>>> >>>>>>> |
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. > >>>>>>> > >>>>>>> > > |
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. >>>>>>>>> >>>>>>>>> >> |
// 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. > >>>>>>>>> > >>>>>>>>> > >> > |
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. >> >>>>>>>>> >> >>>>>>>>> >> >> >> > |
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 |
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 > |
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 |
Free forum by Nabble | Edit this page |