Skip to content

[SPARK-13596] [BUILD] Move misc top-level build files into appropriate subdirs #11522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
srowen wants to merge 5 commits into from
Closed

[SPARK-13596] [BUILD] Move misc top-level build files into appropriate subdirs #11522

srowen wants to merge 5 commits into from

Conversation

@srowen
Copy link
Member

@srowen srowen commented Mar 4, 2016

What changes were proposed in this pull request?

Move many top-level files in dev/ or other appropriate directory. In particular, put make-distribution.sh in dev and update docs accordingly. Remove deprecated sbt/sbt.

I was (so far) unable to figure out how to move tox.ini. scalastyle-config.xml should be movable but edits to the project .sbt files didn't work; config file location is updatable for compile but not test scope.

How was this patch tested?

./dev/run-tests to verify RAT and checkstyle work. Jenkins tests for the rest.

@srowen
Move many top-level files in dev/ or other appropriate directory. In …
4e5ec1e 
…particular, put make-distribution.sh in dev and update docs accordingly. Remove deprecated sbt/sbt
@SparkQA
Copy link

SparkQA commented Mar 4, 2016

Test build #52471 has finished for PR 11522 at commit 4e5ec1e.

  • This patch fails RAT tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • # mixin class is detected if its name ends with \"mixin\" (case insensitive).
    • # Maximum number of parents for a class (see R0901).
    • # Maximum number of attributes for a class (see R0902).
    • # Minimum number of public methods for a class (see R0903).
    • # Maximum number of public methods for a class (see R0904).

nchammas
nchammas reviewed Mar 4, 2016
View reviewed changes
dev/make-distribution.sh

# Figure out where the Spark framework is installed
SPARK_HOME="$(cd "`dirname "$0"`"; pwd)"
SPARK_HOME="$(cd "`dirname "$0"`/.."; pwd)"
Copy link
Contributor

@nchammas nchammas Mar 4, 2016

Choose a reason for hiding this comment

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

Just a heads up: Moving this file is a breaking change for tools like spark-ec2 and Flintrock which automatically build Spark.

I think it's totally OK since 1) this is a developer feature, and 2) this is part of the 2.0 update.

Just wanted to call this out for the record.

Copy link
Member Author

@srowen srowen Mar 4, 2016

Choose a reason for hiding this comment

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

Yeah, I'm even happy to not do this one. It just bugged me that this file was in the top level. It could be symlinked but that doesn't save much. Yeah I suspect some tools would need to look for it in both locations.

Copy link
Contributor

@rxin rxin Mar 7, 2016

Choose a reason for hiding this comment

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

Can we update those tools to look for the file in both? It also really bugs me that this is a top level file ...

Copy link
Contributor

@nchammas nchammas Mar 7, 2016

Choose a reason for hiding this comment

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

Sure. I don't mind opening a PR against spark-ec2 to do this.

Copy link
Contributor

@nchammas nchammas Mar 7, 2016

Choose a reason for hiding this comment

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

Oh never mind, spark-ec2 actually doesn't use make-distribution.sh, so this shouldn't be a problem.

@srowen
Move tox.ini to dev (and update pep8 while we're at it); fix newline …
fdf1cba 
…at end of .rat-excludes; make target dir for RAT first
@SparkQA
Copy link

SparkQA commented Mar 4, 2016

Test build #52475 has finished for PR 11522 at commit fdf1cba.

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

@rxin
Copy link
Contributor

rxin commented Mar 7, 2016

Can we not move the contributing.md? I think people actually read that file directly outside of github.

@srowen
Copy link
Member Author

srowen commented Mar 7, 2016

I'm OK with not moving CONTRIBUTING.md. It can live in .github now and I figured it was another file saved in the top level dir. But I don't mind making it excessively visible.

@SparkQA
Copy link

SparkQA commented Mar 7, 2016

Test build #52548 has finished for PR 11522 at commit 2c1981d.

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

@srowen
Copy link
Member Author

srowen commented Mar 7, 2016

Jenkins retest this please

@SparkQA
Copy link

SparkQA commented Mar 7, 2016

Test build #52556 has finished for PR 11522 at commit 2c1981d.

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

@rxin
Copy link
Contributor

rxin commented Mar 7, 2016

Rest of the changes LGTM.

@srowen can you make sure the style checker rules etc are actually in effect? Maybe try break something in this pr and make sure Jenkins actually fails.

@SparkQA
Copy link

SparkQA commented Mar 7, 2016

Test build #52585 has finished for PR 11522 at commit 0b9bc68.

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

@srowen
Copy link
Member Author

srowen commented Mar 7, 2016

Yeah looks like it still caught the style violation -- see above

@SparkQA
Copy link

SparkQA commented Mar 7, 2016

Test build #52588 has finished for PR 11522 at commit 8e96a7d.

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

@rxin
Copy link
Contributor

rxin commented Mar 7, 2016

Thanks - merging in master.

@asfgit asfgit closed this in 0eea12a Mar 7, 2016
@srowen srowen mentioned this pull request Mar 8, 2016
Closed
@srowen srowen deleted the SPARK-13596 branch March 8, 2016 11:46
roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
@srowen @roygao94
[SPARK-13596][BUILD] Move misc top-level build files into appropriate…
1f6c181 
… subdirs

## What changes were proposed in this pull request?

Move many top-level files in dev/ or other appropriate directory. In particular, put `make-distribution.sh` in `dev` and update docs accordingly. Remove deprecated `sbt/sbt`.

I was (so far) unable to figure out how to move `tox.ini`. `scalastyle-config.xml` should be movable but edits to the project `.sbt` files didn't work; config file location is updatable for compile but not test scope.

## How was this patch tested?

`./dev/run-tests` to verify RAT and checkstyle work. Jenkins tests for the rest.

Author: Sean Owen <sowen@cloudera.com>

Closes apache#11522 from srowen/SPARK-13596.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@srowen @SparkQA @rxin @nchammas