Skip to content

Conversation

@vanzin
Copy link
Contributor

@vanzin vanzin commented Aug 15, 2017

This version fixes a few issues in the import order checker; it provides
better error messages, and detects more improper ordering (thus the need
to change a lot of files in this patch). The main fix is that it correctly
complains about the order of packages vs. classes.

As part of the above, I moved some "SparkSession" import in ML examples
inside the "$example on$" blocks; that didn't seem consistent across
different source files to start with, and avoids having to add more on/off blocks
around specific imports.

The new scalastyle also seems to have a better header detector, so a few
license headers had to be updated to match the expected indentation.

This version fixes a few issues in the import order checker; it provides
better error messages, and detects more improper ordering (thus the need
to change a lot of files in this patch). The main fix is that it correctly
complains about the order of packages vs. classes.

As part of the above, I moved some "SparkSession" import in ML examples
inside the "$example on$" blocks; that didn't seem consistent across
different source files, and avoids having to add more on/off blocks
around specific imports.

It seems to also have a better header detector, so a few license headers
had to be updated to match the expected indentation.
@vanzin
Copy link
Contributor Author

vanzin commented Aug 15, 2017

Will also test with maven once sbt tests pass.

@SparkQA
Copy link

SparkQA commented Aug 15, 2017

Test build #80650 has finished for PR 18943 at commit d38e7dd.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 15, 2017

Argh, forgot to update the tests...

@SparkQA
Copy link

SparkQA commented Aug 15, 2017

Test build #80652 has finished for PR 18943 at commit b1e49fa.

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

// $example on$
import org.apache.spark.ml.feature.BucketedRandomProjectionLSH
import org.apache.spark.ml.linalg.Vectors
import org.apache.spark.sql.SparkSession
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought SparkSession was intentionally hidden in examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be in a few examples; but it's not consistent, and this keeps the code cleaner, even if the import is redundant on a spark-shell session.

@vanzin vanzin changed the title [SPARK-21731][build] Upgrade scalastyle to 0.9. [SPARK-21731][build][test-maven] Upgrade scalastyle to 0.9. Aug 15, 2017
@vanzin
Copy link
Contributor Author

vanzin commented Aug 15, 2017

retest this please

@SparkQA
Copy link

SparkQA commented Aug 15, 2017

Test build #80692 has finished for PR 18943 at commit b1e49fa.

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

@vanzin vanzin changed the title [SPARK-21731][build][test-maven] Upgrade scalastyle to 0.9. [SPARK-21731][build] Upgrade scalastyle to 0.9. Aug 15, 2017
@vanzin
Copy link
Contributor Author

vanzin commented Aug 15, 2017

I'm going to push this to avoid having to re-test it before pushing later after more commits are checked in; I tested on the top of master now and there are no new files that need fixing.

Merging to master.

@asfgit asfgit closed this in 3f958a9 Aug 15, 2017
@vanzin vanzin deleted the SPARK-21731 branch August 15, 2017 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants