-
Notifications
You must be signed in to change notification settings - Fork 252
chore: drop sbt-spark-packages && update plugins #487
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.
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
Conversation
+ drop sbt-spark-packages + rewrite build.sbt in the modern way + update plugins
@rjurney Could you please approve the run of CI? Everything should work in theory, so I would like to see. Thanks! |
ThisBuild / scalaVersion := scalaVer | ||
ThisBuild / organization := "org.graphframes" | ||
|
||
lazy val root = (project in file(".")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need this for adding Spark Connect support: connect 100% should be a separate subproject and it will be much easier to write (project in file("connect")).dependsOn(root)
to inherit most of the settings and use graphframes
(core) as a dependency. As a bonus it is more modern sbt syntax that is recommended by their documentation (instead of implicit project in root)
Calling on @SauronShepherd, @rxin @WeichenXu123, @mengxr, @bjornjorgensen and others better at Scala builds than me to review this. Would be great if we can be timely to encourage @SemyonSinchenko in his new contribution :) |
SemyonSinchenko is a well know person in the spark community.. run the tests, and see whats it say.. |
@SemyonSinchenko approved, thanks for getting involved! |
@SemyonSinchenko lazy val sparkVer = sys.props.getOrElse("spark.version", "3.5.3") we are running 3.5.4
|
It looks like I finally found the subset of plugin versions that works: Everything else should work exactly like it was before. From now we have 1.9.3 sbt that simplify the subprojects creation. @rjurney Could you please approve the run of CI from the latest commit? NEW CHANGES:
P.S. I have no idea why scoverage 2.0.11 did not work but 2.0.10 works. |
@rxin how can we make it so @SemyonSinchenko doesn't need my approval to run CI? I am not sure. |
By default it won't be necessary after the first contribution. |
I'm not a Scala developer. But this looks like a good start to upgrade this project. we should have done something with
it also gives some messages in the build and testing.. like I think you can merge it, its a grate improvement. |
@SemyonSinchenko is this ready to merge? If so I will go for it! |
@SauronShepherd can you plz review? |
Wow, it looks like you had a great time! There are a lot of changes, but I assume they were all necessary. It looks fine to me, though I'm not an SBT expert. |
Okay lemme test real quick and I'll merge! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running ze tests...
# Looking good...
build/sbt clean compile package test
# It assembeles...
build/sbt assembly
Okay, merging...
Close #485