// 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 <ches...@apache.org>
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 <twal...@apache.org>
> >
> > 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 <ches...@apache.org>
> 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 <
> >> pomperma...@okkam.it>
> >>> wrote:
> >>>
> >>>> Thanks Aljoscha and Chesnay for this small but important improvement!
> >>>> In the new year writing new Flink features will be funnier than ever
> ;)
> >>>>
> >>>> On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann <trohrm...@apache.org>
> >>>> 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 <
> matth...@ververica.com>
> >>>>> 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 <imj...@gmail.com> wrote:
> >>>>>>
> >>>>>>> Thanks Aljoscha and Chesnay for the great work!
> >>>>>>>
> >>>>>>> Best,
> >>>>>>> Jark
> >>>>>>>
> >>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song <tonysong...@gmail.com>
> >>>>>> wrote:
> >>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this.
> >>>>>>>>
> >>>>>>>> Thank you~
> >>>>>>>>
> >>>>>>>> Xintong Song
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler <
> >>>> ches...@apache.org
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Hello everyone,
> >>>>>>>>>
> >>>>>>>>> I have just merged the commits for FLINK-20651
> >>>>>>>>> <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 via email to