-
Notifications
You must be signed in to change notification settings - Fork 17
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
DRAFT: Scala3 cross-compilation; #657
base: master
Are you sure you want to change the base?
Conversation
add scala3 cross-compilation; copy single-file hostname library;
add scala3 cross-compilation; copy single-file hostname library; use pureconfig-core; replace Scodec.TransforSyntax.as calls with explicit instances that are compatible between scala2 and scala3 versions;
@@ -7,17 +7,35 @@ lazy val commonSettings = Seq( | |||
organizationHomepage := Some(url("https://evolution.com")), | |||
homepage := Some(url("https://github.com/evolution-gaming/kafka-journal")), | |||
startYear := Some(2018), | |||
crossScalaVersions := Seq("2.13.14"), | |||
crossScalaVersions := Seq("2.13.14", "3.3.3"), |
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.
please update matrix in ci.yml
file too
@@ -16,7 +18,7 @@ object FromConfigReaderResult { | |||
|
|||
implicit def lift[F[_]: ApplicativeThrowable]: FromConfigReaderResult[F] = { | |||
new FromConfigReaderResult[F] { | |||
def apply[A](a: ConfigReader.Result[A]) = { | |||
def apply[A: ClassTag](a: ConfigReader.Result[A]): F[A] = { |
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.
why we need ClassTag
here?
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.
the version of pureconfig that was published for Scala 3, some of its APIs were updated and now require ClassTag
import scala.util.Properties | ||
import scala.util.control.NonFatal | ||
|
||
/** Copied from https://github.com/evolution-gaming/hostname */ |
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.
is this temporary copy till hostname
gets Scala 3 artifacts?
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.
Both yes and no.
Some of evo single-class libraries make almost no sense to me, so I decided to put this one here, especially since there's a wrapper for it anyway.
Also I was not able to build the hostname
easily, there are some weird problems with JVM and SBT or something like that.
If you feel like it should stay as a dependency, I'll update the library. But if you agree on it making no sense, then I'll refactor this file and make it nice.
cancelRef.getAndSet(true) | ||
.flatMap(cancel => | ||
(if (cancel) ().pure[F] else fiber.joinWithNever.void) | ||
|
||
) |
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.
why replace for comprehension with .flatMap
?
I can see usage of Concurrent[F].when(cancel) {fiber.joinWithNever}
as possible improvement, but why structural code change?
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.
It's forced by a weird scala 3 typing-in-for-compresensions issue. Basically the old version can't infer types in the last syntetic map (.map(x => x)
)
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.
so I just did Desugar for-comprehension
in IDEA and removed that last .map
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.
Perhaps it might compile with .void that I also put there, I'll check
objCur <- cur.asObjectCursor | ||
appendCur = objCur.atKeyOrUndefined("append") | ||
append <- if (appendCur.isUndefined) Right(CallTimeThresholds().append) else fdReader.from(appendCur) | ||
|
||
readCur = objCur.atKeyOrUndefined("read") | ||
read <- if (readCur.isUndefined) Right(CallTimeThresholds().read) else fdReader.from(readCur) | ||
|
||
pointerCur = objCur.atKeyOrUndefined("pointer") | ||
pointer <- if (pointerCur.isUndefined) Right(CallTimeThresholds().pointer) else fdReader.from(pointerCur) | ||
|
||
deleteCur = objCur.atKeyOrUndefined("delete") | ||
delete <- if (deleteCur.isUndefined) Right(CallTimeThresholds().delete) else fdReader.from(deleteCur) | ||
|
||
purgeCur <- objCur.atKeyOrUndefined("purge") | ||
purge <- if (purgeCur.isUndefined) Right(CallTimeThresholds().purge) else fdReader.from(purgeCur) | ||
} yield CallTimeThresholds(append, read, pointer, delete, purge) |
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.
please elaborate why this is required
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.
that's the DRAFT part. I'm not sure how to proceed with the config codec derivation since cross-compilation doesn't work with macros. This one is here for now to get the subproject to compile
val `akka-serialization` = "com.evolutiongaming" %% "akka-serialization" % "1.0.6-SNAPSHOT" | ||
val `kafka-launcher` = "com.evolutiongaming" %% "kafka-launcher" % "0.2.0" | ||
val `testcontainers-cassandra` = "com.dimafeng" %% "testcontainers-scala-cassandra" % "0.41.4" | ||
val hostname = "com.evolutiongaming" %% "hostname" % "0.2.0" | ||
val `testcontainers-cassandra` = "com.dimafeng" %% "testcontainers-scala-cassandra" % "0.40.17" | ||
val `testcontainers-kafka` = "com.dimafeng" %% "testcontainers-scala-kafka" % "0.40.17" | ||
val scassandra = "com.evolutiongaming" %% "scassandra" % "5.2.1" | ||
val `cassandra-sync` = "com.evolutiongaming" %% "cassandra-sync" % "3.0.0" | ||
val `cassandra-sync` = "com.evolutiongaming" %% "cassandra-sync" % "3.0.1-SNAPSHOT" | ||
val `cats-helper` = "com.evolutiongaming" %% "cats-helper" % "3.11.0" | ||
val random = "com.evolutiongaming" %% "random" % "1.0.0" | ||
val retry = "com.evolutiongaming" %% "retry" % "3.0.1" | ||
val sstream = "com.evolutiongaming" %% "sstream" % "1.0.1" | ||
val skafka = "com.evolutiongaming" %% "skafka" % "16.0.0" | ||
val random = "com.evolution" %% "random" % "1.0.4" | ||
val retry = "com.evolutiongaming" %% "retry" % "3.1.0" | ||
val sstream = "com.evolutiongaming" %% "sstream" % "1.0.2" | ||
val skafka = "com.evolutiongaming" %% "skafka" % "16.4.0" | ||
val `akka-test-actor` = "com.evolutiongaming" %% "akka-test-actor" % "0.1.0" | ||
val scache = "com.evolution" %% "scache" % "5.0.0" | ||
val `resource-pool` = "com.evolution" %% "resource-pool" % "1.0.4" | ||
val scache = "com.evolution" %% "scache" % "5.1.2" | ||
val `resource-pool` = "com.evolution" %% "resource-pool" % "1.0.5-SNAPSHOT" |
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 suppose those *-SNAPSHOT
dependencies are only for local development, right?
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.
please take in account that some libraries might bring in some transitively incompatible dependencies - please be careful
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, I have a tree of those dependencies written, there will be more MRs.
No description provided.