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