[ 
https://issues.apache.org/jira/browse/FLINK-6107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15985221#comment-15985221
 ] 

Stephan Ewen commented on FLINK-6107:
-------------------------------------

I would like to make a suggestion for an adjustment to the style, specifically 
to the import order.

Basically all other files in Flink have the pattern:
  # Flink / library imports
  # java.foo.bar imports
  # static imports

Also, most files place spaces between components that the imports are derived 
from.

It may just be me, being an oldschool guy that looks at the imports quite a bit 
(almost for every new class I open, I find it useful to get an initial overview 
of what a class interacts with), but I find the style before was much better to 
get a "quick glance" impression:

  - Spaces between logical goups (Flink / logger / library / etc) and 
  - Flink and libraries first (its what matters to get the overview)
  - java below (not really important for the overview)
  - static imports last (they are just syntactic sugar and not required for any 
understanting).

So, why don't we keep that style? It would also result in fewer necessary 
reformatting and fewer merge conflicts.
I pasted the examples below to illustrate that:

h4. Original formatting of most files
{code}
import org.apache.flink.annotation.Internal;
import org.apache.flink.runtime.checkpoint.CheckpointMetaData;
import org.apache.flink.runtime.checkpoint.CheckpointMetrics;
import org.apache.flink.runtime.io.disk.iomanager.IOManager;
import org.apache.flink.runtime.io.network.api.CancelCheckpointMarker;
import org.apache.flink.runtime.io.network.api.CheckpointBarrier;
import org.apache.flink.runtime.io.network.api.EndOfPartitionEvent;
import org.apache.flink.runtime.io.network.partition.consumer.BufferOrEvent;
import org.apache.flink.runtime.io.network.partition.consumer.InputGate;
import org.apache.flink.runtime.jobgraph.tasks.StatefulTask;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.ArrayDeque;

import static org.apache.flink.util.Preconditions.checkArgument;
{code}

h4. New Format
{code}
import static org.apache.flink.util.Preconditions.checkArgument;

import java.io.IOException;
import java.util.ArrayDeque;
import org.apache.flink.annotation.Internal;
import org.apache.flink.runtime.checkpoint.CheckpointMetaData;
import org.apache.flink.runtime.checkpoint.CheckpointMetrics;
import org.apache.flink.runtime.io.disk.iomanager.IOManager;
import org.apache.flink.runtime.io.network.api.CancelCheckpointMarker;
import org.apache.flink.runtime.io.network.api.CheckpointBarrier;
import org.apache.flink.runtime.io.network.api.EndOfPartitionEvent;
import org.apache.flink.runtime.io.network.partition.consumer.BufferOrEvent;
import org.apache.flink.runtime.io.network.partition.consumer.InputGate;
import org.apache.flink.runtime.jobgraph.tasks.StatefulTask;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
{code}

> Add custom checkstyle for flink-streaming-java
> ----------------------------------------------
>
>                 Key: FLINK-6107
>                 URL: https://issues.apache.org/jira/browse/FLINK-6107
>             Project: Flink
>          Issue Type: Improvement
>          Components: DataStream API
>            Reporter: Aljoscha Krettek
>            Assignee: Aljoscha Krettek
>             Fix For: 1.3.0
>
>
> There was some consensus on the ML 
> (https://lists.apache.org/thread.html/94c8c5186b315c58c3f8aaf536501b99e8b92ee97b0034dee295ff6a@%3Cdev.flink.apache.org%3E)
>  that we want to have a more uniform code style. We should start 
> module-by-module and by introducing increasingly stricter rules. We have to 
> be aware of the PR situation and ensure that we have minimal breakage for 
> contributors.
> This issue aims at adding a custom checkstyle.xml for 
> {{flink-streaming-java}} that is based on our current checkstyle.xml but adds 
> these checks for Javadocs:
> {code}
> <!--
> JAVADOC CHECKS
> -->
> <!-- Checks for Javadoc comments.                     -->
> <!-- See http://checkstyle.sf.net/config_javadoc.html -->
> <module name="JavadocMethod">
>   <property name="scope" value="protected"/>
>   <property name="severity" value="error"/>
>   <property name="allowMissingJavadoc" value="true"/>
>   <property name="allowMissingParamTags" value="true"/>
>   <property name="allowMissingReturnTag" value="true"/>
>   <property name="allowMissingThrowsTags" value="true"/>
>   <property name="allowThrowsTagsForSubclasses" value="true"/>
>   <property name="allowUndeclaredRTE" value="true"/>
> </module>
> <!-- Check that paragraph tags are used correctly in Javadoc. -->
> <module name="JavadocParagraph"/>
> <module name="JavadocType">
>   <property name="scope" value="protected"/>
>   <property name="severity" value="error"/>
>   <property name="allowMissingParamTags" value="true"/>
> </module>
> <module name="JavadocStyle">
>   <property name="severity" value="error"/>
>   <property name="checkHtml" value="true"/>
> </module>
> {code}
> This checks:
>  - Every type has a type-level Javadoc
>  - Proper use of {{<p>}} in Javadocs
>  - First sentence must end with a proper punctuation mark
>  - Proper use (including closing) of HTML tags



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to