You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2017/05/23 16:38:44 UTC

[GitHub] flink pull request #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

GitHub user zentol opened a pull request:

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

    [FLINK-6687] [web] Activate strict checkstyle for flink-runtime-web

    Lot's of missing javadocs, trailing spaces in javadocs, trailing tabs and missing dots.
    
    Very little code is affected.

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

    $ git pull https://github.com/zentol/flink 6687_checkstyle_web

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

    https://github.com/apache/flink/pull/3973.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 #3973
    
----
commit b680a445d17e2f9c55ea6bed7319e93ca1982e8e
Author: zentol <ch...@apache.org>
Date:   2017-05-23T16:36:51Z

    [FLINK-6687] [web] Activate strict checkstyle for flink-runtime-web

----


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118056115
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/RuntimeMonitorHandler.java ---
    @@ -29,15 +34,8 @@
     import io.netty.handler.codec.http.HttpVersion;
     import io.netty.handler.codec.http.router.KeepAliveWrite;
     import io.netty.handler.codec.http.router.Routed;
    -
    -import org.apache.flink.configuration.ConfigConstants;
    -import org.apache.flink.runtime.instance.ActorGateway;
    -import org.apache.flink.runtime.webmonitor.handlers.RequestHandler;
    -import org.apache.flink.util.ExceptionUtils;
    -
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
    -
    --- End diff --
    
    We should consider giving `scala` its own block as with `java` and `javax`.


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118075328
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/RequestHandler.java ---
    @@ -36,13 +37,15 @@
     	 * respond with a full http response, including content-type, content-length, etc.
     	 *
     	 * <p>Exceptions may be throws and will be handled.
    -	 * 
    +	 *
    +*
    --- End diff --
    
    remnants of a regex replacement gone wrong :/


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for flink-r...

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

    https://github.com/apache/flink/pull/3973
  
    +1


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for flink-r...

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

    https://github.com/apache/flink/pull/3973
  
    @greghogan I've added a separate import block for scala and addressed all comments.


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

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


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for flink-r...

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

    https://github.com/apache/flink/pull/3973
  
    I'll merge this after the metrics/python/annotations checkstyle PR's are merged. Need to rebase due to the newly added rule.


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118071671
  
    --- Diff: flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/metrics/TaskManagerMetricsHandlerTest.java ---
    @@ -33,6 +34,9 @@
     import static org.junit.Assert.assertNull;
     import static org.powermock.api.mockito.PowerMockito.mock;
     
    +/**
    + * Tests for the TaskManagersMetricsHandler.
    --- End diff --
    
    `Manager`


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118057623
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/RequestHandler.java ---
    @@ -36,13 +37,15 @@
     	 * respond with a full http response, including content-type, content-length, etc.
     	 *
     	 * <p>Exceptions may be throws and will be handled.
    -	 * 
    +	 *
    +*
    --- End diff --
    
    Indentation lines 41 and 48.


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118075403
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/RuntimeMonitorHandler.java ---
    @@ -29,15 +34,8 @@
     import io.netty.handler.codec.http.HttpVersion;
     import io.netty.handler.codec.http.router.KeepAliveWrite;
     import io.netty.handler.codec.http.router.Routed;
    -
    -import org.apache.flink.configuration.ConfigConstants;
    -import org.apache.flink.runtime.instance.ActorGateway;
    -import org.apache.flink.runtime.webmonitor.handlers.RequestHandler;
    -import org.apache.flink.util.ExceptionUtils;
    -
     import org.slf4j.Logger;
     import org.slf4j.LoggerFactory;
    -
    --- 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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118071367
  
    --- Diff: flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/handlers/checkpoints/CheckpointStatsSubtaskDetailsHandlerTest.java ---
    @@ -57,6 +57,9 @@
     import static org.mockito.Mockito.verify;
     import static org.mockito.Mockito.when;
     
    +/**
    + * Tests for the CheckpointStatsSubtaskDetailsHandlerTest.
    --- End diff --
    
    Self-reference


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118058165
  
    --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/TaskManagerLogHandler.java ---
    @@ -100,26 +101,27 @@
     	private static final String TASKMANAGER_LOG_REST_PATH = "/taskmanagers/:taskmanagerid/log";
     	private static final String TASKMANAGER_OUT_REST_PATH = "/taskmanagers/:taskmanagerid/stdout";
     
    -	/** Keep track of last transmitted log, to clean up old ones */
    +	/** Keep track of last transmitted log, to clean up old ones. */
     	private final HashMap<String, BlobKey> lastSubmittedLog = new HashMap<>();
     	private final HashMap<String, BlobKey> lastSubmittedStdout = new HashMap<>();
     
    -	/** Keep track of request status, prevents multiple log requests for a single TM running concurrently */
    +	/** Keep track of request status, prevents multiple log requests for a single TM running concurrently. */
     	private final ConcurrentHashMap<String, Boolean> lastRequestPending = new ConcurrentHashMap<>();
     	private final Configuration config;
     
    -	/** Future of the blob cache */
    +	/** Future of the blob cache. */
     	private Future<BlobCache> cache;
     
    -	/** Indicates which log file should be displayed; true indicates .log, false indicates .out */
    -	private boolean serveLogFile;
    +	/** Indicates which log file should be displayed;. */
    --- End diff --
    
    Unnecessary semicolon.


---
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 #3973: [FLINK-6687] [web] Activate strict checkstyle for ...

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

    https://github.com/apache/flink/pull/3973#discussion_r118060440
  
    --- Diff: flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/metrics/MetricStoreTest.java ---
    @@ -15,17 +15,22 @@
      * See the License for the specific language governing permissions and
      * limitations under the License.
      */
    +
     package org.apache.flink.runtime.webmonitor.metrics;
     
     import org.apache.flink.runtime.metrics.dump.MetricDump;
     import org.apache.flink.runtime.metrics.dump.QueryScopeInfo;
     import org.apache.flink.util.TestLogger;
    +
     import org.junit.Test;
     
     import java.io.IOException;
     
     import static org.junit.Assert.assertEquals;
     
    +/**
    + * Tests for teh MetricStore.
    --- End diff --
    
    spelling


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