-
Notifications
You must be signed in to change notification settings - Fork 645
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
Scala3 builds csv, cassandra, testkit #2957
Conversation
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.
Do we need to add something in ci config to compile the supported modules with Scala 3 on PR/nightlies as well?
I am not sure if we run nightlies, do we? At least I did not see it. Re scala3 in PRs, I got the understanding that we only want scala3 (same for 2.12) to be used during cros publishing. At least this is how it is right now for the scla3 builds I was working on. |
@@ -7,7 +7,9 @@ object Dependencies { | |||
|
|||
val Scala213 = "2.13.10" // update even in link-validator.conf | |||
val Scala212 = "2.12.17" | |||
val ScalaVersions = Seq(Scala213, Scala212) | |||
val Scala3 = "3.1.3" | |||
val Scala2Versions = Seq(Scala213, Scala212) |
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.
Re scala3 in PRs, I got the understanding that we only want scala3 (same for 2.12) to be used during cros publishing. At least this is how it is right now for the scla3 builds I was working on.
We should at minimum compile with all Scala versions. We typically do that with +Test/compile
. That should be changed here
run: sbt +~2.13 "verifyCodeStyle; Test/compile; mimaReportBinaryIssues" |
It's probably obsolete to use 2.13 there, since 2.13 is the default version.
Also, currently I think we run tests with the default 2.13, which is good, but it means that we currently don't build with 2.12 at all (unless I missed something, didn't look so close). Anyway, that is fixed by compiling with +Test/compile
.
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.
Makes sense, will review.
Something is off with CI, it does not succeed and I can not cancel it... |
Looks like that worked nicely (I see 3.1 in the |
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.
LGTM!
I tried out a +publishLocal to double check that package/docs work fine as well and that looks good.
No description provided.