You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by greghogan <gi...@git.apache.org> on 2017/07/10 22:44:03 UTC

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

GitHub user greghogan opened a pull request:

    https://github.com/apache/flink/pull/4295

    [FLINK-6731] [tests] Activate strict checkstyle for flink-tests

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/greghogan/flink 6731_activate_strict_checkstyle_for_flink_tests

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/4295.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4295
    
----
commit 58c9c8ae9643145b17ccd87270729c4ed1cd06ff
Author: Greg Hogan <co...@greghogan.com>
Date:   2017-05-30T19:40:47Z

    optimize imports

commit 62b8f4d6eccbec00bdb70cafbc4aef7281f16efe
Author: Greg Hogan <co...@greghogan.com>
Date:   2017-05-30T19:42:37Z

    trailing whitespace

commit 366b6c047d59545ce45ec62471f15ac3c662bc6d
Author: Greg Hogan <co...@greghogan.com>
Date:   2017-05-30T20:02:00Z

    additional updates

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126928351
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/misc/SuccessAfterNetworkBuffersFailureITCase.java ---
    @@ -168,6 +173,6 @@ private static void runKMeans(ExecutionEnvironment env) throws Exception {
     
     		clusteredPoints.output(new DiscardingOutputFormat<Tuple2<Integer, KMeans.Point>>());
     
    -		env.execute("KMeans Example");
    +		env.execute("kmeans Example");
    --- End diff --
    
    Agreed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126949837
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/KMeansWithBroadcastSetITCase.java ---
    @@ -65,7 +68,7 @@ public Centroid map(String c) {
     					}
     				});
     
    -		// set number of bulk iterations for KMeans algorithm
    +		// set number of bulk iterations for kmeans algorithm
    --- End diff --
    
    I reverted these and the additional instances. It looks to have happened when renaming `KMeansSingleStepTest#KMeans`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126907648
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/accumulators/AccumulatorIterativeITCase.java ---
    @@ -30,6 +29,9 @@
     
     import org.junit.Assert;
     
    +/**
    + *
    --- End diff --
    
    empty javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126908782
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/StreamCheckpointNotifierITCase.java ---
    @@ -324,7 +322,7 @@ public void notifyCheckpointComplete(long checkpointId) {
     	private static class LeftIdentityCoRichFlatMapFunction extends RichCoFlatMapFunction<Long, Long, Long>
     			implements CheckpointListener {
     
    -		static final List<Long>[] completedCheckpoints = createCheckpointLists(PARALLELISM);
    --- End diff --
    
    did you intentionally remove `final`? (there are other instances of change)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/4295


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4295: [FLINK-6731] [tests] Activate strict checkstyle for flink...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/4295
  
    +1 with green light from travis.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126947445
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/KMeansWithBroadcastSetITCase.java ---
    @@ -65,7 +68,7 @@ public Centroid map(String c) {
     					}
     				});
     
    -		// set number of bulk iterations for KMeans algorithm
    +		// set number of bulk iterations for kmeans algorithm
    --- End diff --
    
    Another instance of "kmeans" renaming


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4295: [FLINK-6731] [tests] Activate strict checkstyle for flink...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/4295
  
    Whitespace changes look ok.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126908486
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/RescalingITCase.java ---
    @@ -1035,4 +1040,43 @@ public void restoreState(Integer state) throws Exception {
     			counterPartitions.add(state);
     		}
     	}
    +
    +	/**
    +	 * Copied from io.netty.util.internal.ConcurrentSet.
    +	 *
    +	 * @param <E> the type of elements maintained by this set
    +	 */
    +	private static final class ConcurrentSet<E> extends AbstractSet<E> implements Serializable {
    --- End diff --
    
    You could also use `Collections.newSetFromMap(new HashMap<>())`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126913993
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/windowing/sessionwindows/GeneratorConfiguration.java ---
    @@ -36,9 +36,9 @@
     	private final long maxAdditionalSessionGap;
     
     	public GeneratorConfiguration(long allowedLateness,
    --- End diff --
    
    let's move the `allowedLateness` argument to the next line as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126928245
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/RescalingITCase.java ---
    @@ -1035,4 +1040,43 @@ public void restoreState(Integer state) throws Exception {
     			counterPartitions.add(state);
     		}
     	}
    +
    +	/**
    +	 * Copied from io.netty.util.internal.ConcurrentSet.
    +	 *
    +	 * @param <E> the type of elements maintained by this set
    +	 */
    +	private static final class ConcurrentSet<E> extends AbstractSet<E> implements Serializable {
    --- End diff --
    
    This dependency was removed in a recent commit so am deleting the class.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126914082
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/windowing/sessionwindows/GeneratorEventFactory.java ---
    @@ -39,9 +39,9 @@
     	 * @return event for an keyed event generator
     	 */
     	E createEvent(K key,
    --- End diff --
    
    move to next line


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126947465
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/classloading/jar/KMeansForTest.java ---
    @@ -70,7 +69,7 @@ public static void main(String[] args) throws Exception {
     		DataSet<Centroid> centroids = env.fromElements(centersData.split("\n"))
     				.map(new TupleCentroidConverter());
     
    -		// set number of bulk iterations for KMeans algorithm
    +		// set number of bulk iterations for kmeans algorithm
    --- End diff --
    
    and another


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126928229
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/accumulators/AccumulatorIterativeITCase.java ---
    @@ -30,6 +29,9 @@
     
     import org.junit.Assert;
     
    +/**
    + *
    --- End diff --
    
    Updated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126928363
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/windowing/sessionwindows/GeneratorConfiguration.java ---
    @@ -36,9 +36,9 @@
     	private final long maxAdditionalSessionGap;
     
     	public GeneratorConfiguration(long allowedLateness,
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126928374
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/windowing/sessionwindows/GeneratorEventFactory.java ---
    @@ -39,9 +39,9 @@
     	 * @return event for an keyed event generator
     	 */
     	E createEvent(K key,
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4295: [FLINK-6731] [tests] Activate strict checkstyle for flink...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/4295
  
    The `pom.xml` wasn't updated to run the checkstyle plugin.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #4295: [FLINK-6731] [tests] Activate strict checkstyle for flink...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/4295
  
    Import changes look ok.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126928311
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/StreamCheckpointNotifierITCase.java ---
    @@ -324,7 +322,7 @@ public void notifyCheckpointComplete(long checkpointId) {
     	private static class LeftIdentityCoRichFlatMapFunction extends RichCoFlatMapFunction<Long, Long, Long>
     			implements CheckpointListener {
     
    -		static final List<Long>[] completedCheckpoints = createCheckpointLists(PARALLELISM);
    --- End diff --
    
    I've fixed the instances in this file. Variables are now uppercase.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4295#discussion_r126911432
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/misc/SuccessAfterNetworkBuffersFailureITCase.java ---
    @@ -168,6 +173,6 @@ private static void runKMeans(ExecutionEnvironment env) throws Exception {
     
     		clusteredPoints.output(new DiscardingOutputFormat<Tuple2<Integer, KMeans.Point>>());
     
    -		env.execute("KMeans Example");
    +		env.execute("kmeans Example");
    --- End diff --
    
    Not sure if we have to change this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---