[SPARK-19227][SPARK-19251] remove unused imports and outdated comments #16591

uncleGen · 2017-01-16T01:56:05Z

What changes were proposed in this pull request?

remove ununsed imports and outdated comments, and fix some minor code style issue.

How was this patch tested?

existing ut

…che.spark.internal.config.ConfigEntry`

SparkQA · 2017-01-16T04:24:08Z

Test build #71414 has finished for PR 16591 at commit 22405d1.

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

HyukjinKwon · 2017-01-16T04:28:44Z

I know this is acceptable assuming from the history. However, I have seen a lot of unused imports across the code base. I think it'd be nicer if the same instances are checked at least within the same package.

uncleGen · 2017-01-16T06:47:39Z

This work does not change any code, but just delete unused imports and fix some code style issue.
cc @srowen

Because there are lots of similar problems in code base, I will split this work into several follow-up PRs.

SparkQA · 2017-01-16T06:48:34Z

Test build #71422 has started for PR 16591 at commit e98d9ab.

uncleGen · 2017-01-16T08:08:24Z

retest this please

SparkQA · 2017-01-16T10:40:56Z

Test build #71431 has finished for PR 16591 at commit e98d9ab.

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

srowen

This seems to be tagged to the wrong JIRA

srowen · 2017-01-16T20:46:20Z

core/src/main/java/org/apache/spark/api/java/JavaFutureAction.java

This type of change shouldn't be made. Removing dead imports we usually don't bother with but maybe one sweep of all the code is OK.

rxin · 2017-01-16T23:30:57Z

core/src/main/scala/org/apache/spark/executor/OutputMetrics.scala

why are you removing the line here? having two blank lines to separate top level classes is actually a style.

SparkQA · 2017-01-17T05:10:00Z

Test build #71470 has finished for PR 16591 at commit 958c2fe.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

3. remove outdated parameter comments

SparkQA · 2017-01-17T06:39:21Z

Test build #71480 has finished for PR 16591 at commit b5244ec.

This patch fails Scala style tests.
This patch does not merge cleanly.
This patch adds no public classes.

uncleGen · 2017-01-17T06:42:43Z

retest this please.

SparkQA · 2017-01-17T06:53:10Z

Test build #71481 has finished for PR 16591 at commit ab70d6b.

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

SparkQA · 2017-01-17T07:03:35Z

Test build #71482 has started for PR 16591 at commit f0e0576.

uncleGen · 2017-01-17T08:08:24Z

No errors, but process was terminated by signal 9.

uncleGen · 2017-01-17T08:25:40Z

retest this please.

SparkQA · 2017-01-17T11:20:55Z

Test build #71493 has finished for PR 16591 at commit f0e0576.

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

srowen

OK, I know we've said don't bother with import fixups before, but I do think the occasional complete sweep is worthwhile, in a minor release.

I had a few comments but I'm OK with merging this for master otherwise.

srowen · 2017-01-17T12:30:07Z

core/src/main/scala/org/apache/spark/rpc/netty/NettyStreamManager.scala

 import java.io.File
 import java.util.concurrent.ConcurrentHashMap

+import org.apache.spark.SparkContext


Was this intentional? I would have guessed this wouldn't compile without this import before if it were needed.

srowen · 2017-01-17T12:31:01Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala

 import org.apache.spark.sql.types.StructType


-


Back this out too if you please to be consistent

srowen · 2017-01-17T12:33:54Z

@uncleGen Oh, I see now that SPARK-19227 is just a doc issue and you fixed it here. That also isn't something that needs a JIRA. But if you resolve it here, also add [SPARK-19227] to the title as well.

SparkQA · 2017-01-18T04:03:02Z

Test build #71547 has finished for PR 16591 at commit a9386cb.

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

srowen · 2017-01-18T09:44:44Z

Merged to master

## What changes were proposed in this pull request? remove ununsed imports and outdated comments, and fix some minor code style issue. ## How was this patch tested? existing ut Author: uncleGen <[email protected]> Closes apache#16591 from uncleGen/SPARK-19227.

SPARK-19227: remove ununsed imports and outdated comments in `org.apa…

22405d1

…che.spark.internal.config.ConfigEntry`

uncleGen changed the title ~~[SPARK-19227][CORE] remove ununsed imports and outdated comments in org.apache.spark.internal.config.ConfigEntry~~ [SPARK-19227][CORE] remove unused imports and outdated comments Jan 16, 2017

srowen requested changes Jan 16, 2017

View reviewed changes

rxin reviewed Jan 16, 2017

View reviewed changes

uncleGen changed the title ~~[SPARK-19227][CORE] remove unused imports and outdated comments~~ [SPARK-19251][CORE] remove unused imports and outdated comments Jan 17, 2017

1. remove unused imports

b5244ec

3. remove outdated parameter comments

uncleGen force-pushed the SPARK-19227 branch from 958c2fe to b5244ec Compare January 17, 2017 06:36

Merge branch 'master' into SPARK-19227

ea6356f

fix code style

ab70d6b

uncleGen changed the title ~~[SPARK-19251][CORE] remove unused imports and outdated comments~~ [SPARK-19251] remove unused imports and outdated comments Jan 17, 2017

revert: not found: type varargs

f0e0576

srowen reviewed Jan 17, 2017

View reviewed changes

uncleGen changed the title ~~[SPARK-19251] remove unused imports and outdated comments~~ [SPARK-19227][SPARK-19251] remove unused imports and outdated comments Jan 18, 2017

revert

a9386cb

asfgit closed this in eefdf9f Jan 18, 2017

[SPARK-19227][SPARK-19251] remove unused imports and outdated comments #16591

[SPARK-19227][SPARK-19251] remove unused imports and outdated comments #16591

Uh oh!

Conversation

uncleGen commented Jan 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Jan 16, 2017

Uh oh!

HyukjinKwon commented Jan 16, 2017

Uh oh!

uncleGen commented Jan 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jan 16, 2017

Uh oh!

uncleGen commented Jan 16, 2017

Uh oh!

SparkQA commented Jan 16, 2017

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

uncleGen Jan 17, 2017

Choose a reason for hiding this comment

Uh oh!

rxin Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 17, 2017

Uh oh!

SparkQA commented Jan 17, 2017

Uh oh!

uncleGen commented Jan 17, 2017

Uh oh!

SparkQA commented Jan 17, 2017

Uh oh!

SparkQA commented Jan 17, 2017

Uh oh!

uncleGen commented Jan 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uncleGen commented Jan 17, 2017

Uh oh!

SparkQA commented Jan 17, 2017

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen Jan 17, 2017

Choose a reason for hiding this comment

Uh oh!

srowen Jan 17, 2017

Choose a reason for hiding this comment

Uh oh!

srowen commented Jan 17, 2017

Uh oh!

SparkQA commented Jan 18, 2017

Uh oh!

srowen commented Jan 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

uncleGen commented Jan 16, 2017 •

edited

Loading

uncleGen commented Jan 16, 2017 •

edited

Loading

uncleGen commented Jan 17, 2017 •

edited

Loading