[MINOR][BUILD] Fix lint-java breaks in Java by HyukjinKwon · Pull Request #17072 · apache/spark

HyukjinKwon · 2017-02-26T06:53:30Z

What changes were proposed in this pull request?

This PR proposes to fix the lint-breaks as below:

[ERROR] src/test/java/org/apache/spark/network/TransportResponseHandlerSuite.java:[29,8] (imports) UnusedImports: Unused import - org.apache.spark.network.buffer.ManagedBuffer.
[ERROR] src/main/java/org/apache/spark/unsafe/types/UTF8String.java:[156,10] (modifier) ModifierOrder: '@Nonnull' annotation modifier does not precede non-annotation modifiers.
[ERROR] src/main/java/org/apache/spark/SparkFirehoseListener.java:[122] (sizes) LineLength: Line is longer than 100 characters (found 105).
[ERROR] src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java:[164,78] (coding) OneStatementPerLine: Only one statement per line allowed.
[ERROR] src/test/java/test/org/apache/spark/JavaAPISuite.java:[1157] (sizes) LineLength: Line is longer than 100 characters (found 121).
[ERROR] src/test/java/org/apache/spark/streaming/JavaMapWithStateSuite.java:[149] (sizes) LineLength: Line is longer than 100 characters (found 113).
[ERROR] src/test/java/test/org/apache/spark/streaming/Java8APISuite.java:[146] (sizes) LineLength: Line is longer than 100 characters (found 122).
[ERROR] src/test/java/test/org/apache/spark/streaming/JavaAPISuite.java:[32,8] (imports) UnusedImports: Unused import - org.apache.spark.streaming.Time.
[ERROR] src/test/java/test/org/apache/spark/streaming/JavaAPISuite.java:[611] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/test/org/apache/spark/streaming/JavaAPISuite.java:[1317] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/test/org/apache/spark/sql/JavaDatasetAggregatorSuite.java:[91] (sizes) LineLength: Line is longer than 100 characters (found 102).
[ERROR] src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:[113] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:[164] (sizes) LineLength: Line is longer than 100 characters (found 110).
[ERROR] src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:[212] (sizes) LineLength: Line is longer than 100 characters (found 114).
[ERROR] src/test/java/org/apache/spark/mllib/tree/JavaDecisionTreeSuite.java:[36] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/main/java/org/apache/spark/examples/streaming/JavaKinesisWordCountASL.java:[26,8] (imports) UnusedImports: Unused import - com.amazonaws.regions.RegionUtils.
[ERROR] src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisStreamSuite.java:[20,8] (imports) UnusedImports: Unused import - com.amazonaws.regions.RegionUtils.
[ERROR] src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisStreamSuite.java:[94] (sizes) LineLength: Line is longer than 100 characters (found 103).
[ERROR] src/main/java/org/apache/spark/examples/ml/JavaTokenizerExample.java:[30,8] (imports) UnusedImports: Unused import - org.apache.spark.sql.api.java.UDF1.
[ERROR] src/main/java/org/apache/spark/examples/ml/JavaTokenizerExample.java:[72] (sizes) LineLength: Line is longer than 100 characters (found 104).
[ERROR] src/main/java/org/apache/spark/examples/mllib/JavaRankingMetricsExample.java:[121] (sizes) LineLength: Line is longer than 100 characters (found 101).
[ERROR] src/main/java/org/apache/spark/examples/sql/JavaSQLDataSourceExample.java:[28,8] (imports) UnusedImports: Unused import - org.apache.spark.api.java.JavaRDD.
[ERROR] src/main/java/org/apache/spark/examples/sql/JavaSQLDataSourceExample.java:[29,8] (imports) UnusedImports: Unused import - org.apache.spark.api.java.JavaSparkContext.

How was this patch tested?

Manually via

./dev/lint-java

HyukjinKwon · 2017-02-26T06:54:28Z

common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java

This seems actually a correct order but there is a bug in checkstyle. Please see https://github.com/checkstyle/checkstyle/issues/903. I am willing to disable ModifierOrder for this method if anyone thinks so.

The compiler accepts it but I agree that annotations conventionally come before modifiers. They usually come on the line above.

HyukjinKwon · 2017-02-26T06:55:10Z

I haven't checked the tests. Maybe, let me wait for the test results before cc'ing someone.

SparkQA · 2017-02-26T07:04:46Z

Test build #73480 has finished for PR 17072 at commit f196744.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-02-26T07:22:30Z

Test build #73481 has started for PR 17072 at commit 666ffed.

HyukjinKwon · 2017-02-26T08:06:54Z

retest this please

srowen · 2017-02-26T09:10:10Z

Thank you, yes just last night I put together some changes to fix this too, because I added some problems when I changed to Java 8 syntax. Maybe you can apply https://github.com/srowen/spark/commit/fcc66a0a79399b81c08c17f93073a9635de90a95.patch to your branch to see if I got anything else?

srowen · 2017-02-26T09:10:48Z

core/src/main/java/org/apache/spark/SparkFirehoseListener.java

    @Override
    public final void onStageCompleted(SparkListenerStageCompleted stageCompleted) {
-        onEvent(stageCompleted);
+      onEvent(stageCompleted);


Yeah, the whole file is mis-indented. The first indent is still 4 spaces. I think you could fix that too if you like

Ah, sure. Thank you. Let me address the comments and compare this to your work too.

SparkQA · 2017-02-26T10:52:39Z

Test build #73482 has finished for PR 17072 at commit 666ffed.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-02-26T14:28:08Z

Test build #73485 has finished for PR 17072 at commit 647457c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-02-26T14:36:30Z

Test build #73486 has finished for PR 17072 at commit 2294a19.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2017-02-27T09:05:16Z

Thank you @srowen.

HyukjinKwon commented Feb 26, 2017

View reviewed changes

HyukjinKwon changed the title ~~[MINOR][BUILD] Fix lint-java breaks in Java~~ [WIP][MINOR][BUILD] Fix lint-java breaks in Java Feb 26, 2017

Fix lint breaks in Java

666ffed

HyukjinKwon force-pushed the java-lint branch from f196744 to 666ffed Compare February 26, 2017 07:19

HyukjinKwon changed the title ~~[WIP][MINOR][BUILD] Fix lint-java breaks in Java~~ [MINOR][BUILD] Fix lint-java breaks in Java Feb 26, 2017

srowen reviewed Feb 26, 2017

View reviewed changes

HyukjinKwon added 2 commits February 26, 2017 20:34

Consolidate suggested works and address comments

647457c

Double check and clean few nits

2294a19

srowen approved these changes Feb 26, 2017

View reviewed changes

asfgit closed this in 4ba9c6c Feb 27, 2017

HyukjinKwon deleted the java-lint branch January 2, 2018 03:38

Conversation

HyukjinKwon commented Feb 26, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

HyukjinKwon Feb 26, 2017

Choose a reason for hiding this comment

Uh oh!

srowen Feb 26, 2017

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Feb 26, 2017

Uh oh!

SparkQA commented Feb 26, 2017

Uh oh!

SparkQA commented Feb 26, 2017

Uh oh!

HyukjinKwon commented Feb 26, 2017

Uh oh!

srowen commented Feb 26, 2017

Uh oh!

srowen Feb 26, 2017

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Feb 26, 2017

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 26, 2017

Uh oh!

SparkQA commented Feb 26, 2017

Uh oh!

SparkQA commented Feb 26, 2017

Uh oh!

HyukjinKwon commented Feb 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants