-
Notifications
You must be signed in to change notification settings - Fork 94
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
breaking: Update to Scala 3.3.3
#507
Conversation
Scala 3 case-app doesn't depend on shapeless anymore, which should reduce the binary size This also updates dependencies to their Scala 3 compatible counterpart - case-app is updated to `2.1.0-M28` - airframe-log is updated to `24.6.0` - airframe-log is updated to `24.6.0` - `mockito` is replaced with `scalamock` which supports scala 3 - `scalatest` is updated to `3.2.18` - `play-json` is updated to `2.10.5` - `codacy-plugins-api` is updated to `8.1.4` - `scala-xml` is updated to `2.3.0`
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences🚀 Don’t miss a bit, follow what’s new on Codacy. Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
@@ -53,9 +53,10 @@ class GitClient(workDirectory: File) { | |||
|
|||
val result: Seq[String] = | |||
if (treeWalk.next) { | |||
Stream | |||
Iterator |
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.
Before this was lazy, but now with the .toSeq
it is not. Maybe we want to use LazyList
here to maintain the semantics?
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.
Good point! Fixing it.
|
||
name := "codacy-coverage-reporter" | ||
|
||
// Runtime dependencies | ||
libraryDependencies ++= Seq( | ||
"com.github.alexarchambault" %% "case-app" % "2.1.0-M26", | ||
"org.wvlet.airframe" %% "airframe-log" % "22.3.0" | ||
"com.github.alexarchambault" %% "case-app" % "2.1.0-M28", |
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.
🙃
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.
🤷♂️
"com.typesafe.play" %% "play-json" % "2.8.2", | ||
"org.scalaj" %% "scalaj-http" % "2.4.2", | ||
"com.typesafe.play" %% "play-json" % "2.10.5", | ||
("org.scalaj" %% "scalaj-http" % "2.4.2").cross(CrossVersion.for3Use2_13), |
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.
What is the ideia here? That lib is not available for Scala3, and with this we are relying on the one from 2.13? (It also seems deprecated, so I guess we will want to swap at some point)
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.
Exactly!
I tried to swap with sttp once and I created an outage, but I will give another try with a newer and better maintained library. Fortunately Sbt can check for incompatibilities with the mix of _2.13
and _3
dependencies, so we are totally safe here.
|
||
gitFileFetcher.forCommit(any[String]).shouldReturn(Right(Seq.empty)) | ||
(gitFileFetcher.forCommit _).when(*).returns(Right(Seq.empty)) |
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.
Such a strange syntax imo, but it is what it is
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.
Yes, Scalamock stretches Scala's syntax to be able to mock at compile time and not using reflection, so I'm happy enough.
def commonSettings = | ||
Seq(scalacOptions := { | ||
val toFilter = Set("-deprecation:false") | ||
scalacOptions.value.filterNot(toFilter) ++ Seq("-deprecation") |
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.
What is happening here and why do we need it?
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.
There were two issues that were failing compilation because of -Xfatal-warnings
and couldn't be seen because of the -deprecation:false
setting in codacy-sbt
. Maybe we need to revisit scalacOptions
in codacy-sbt but until that day this removes that option and adds -deprecation
.
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!
Scala 3 case-app doesn't depend on shapeless anymore, which should reduce the binary size
This also updates dependencies to their Scala 3 compatible counterpart
case-app
is updated to2.1.0-M28
airframe-log
is updated to24.6.0
mockito
is replaced withscalamock
which supports scala 3scalatest
is updated to3.2.18
play-json
is updated to2.10.5
codacy-plugins-api
is updated to8.1.4
scala-xml
is updated to2.3.0