Scala 3 support - Githubissues

jtjeferreira commented 9 months ago

Adds scala 3 support

lightbend-cla-validator commented 9 months ago

Hi @jtjeferreira,

Thank you for your contribution! We really value the time you've taken to put this together.

We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it. Please review the new CLA and sign it before we proceed with reviewing this pull request:

http://www.lightbend.com/contribute/cla

johanandren commented 5 months ago

Hey @jtjeferreira any chance you have time to rebase on master to solve the conflict so we can see if Slick 3.5.0 RC1 works?

jtjeferreira commented 5 months ago

Hey @jtjeferreira any chance you have time to rebase on master to solve the conflict so we can see if Slick 3.5.0 RC1 works?

I tried the "Resolve Conflicts" from the GitHub UI on my phone and the conflict seems simple. I will have a look tomorrow...

jtjeferreira commented 5 months ago

Hey @jtjeferreira any chance you have time to rebase on master to solve the conflict so we can see if Slick 3.5.0 RC1 works?

I tried the "Resolve Conflicts" from the GitHub UI on my phone and the conflict seems simple. I will have a look tomorrow...

I fixed the conflicts. Can you approve the workflows?

PS: There is a test that will fail, due to a compiler bug

[info] H2ScalaEventsByTagMigrationTest:
[info] - should migrate event tag to new way *** FAILED *** (2 milliseconds)
[info]   java.lang.ClassCastException: class com.typesafe.config.impl.SimpleConfig cannot be cast to class java.lang.String (com.typesafe.config.impl.SimpleConfig is in unnamed module of loader sbt.internal.LayeredClassLoader @611cd810; java.lang.String is in module java.base of loader 'bootstrap')
[info]   at akka.persistence.jdbc.query.EventsByTagMigrationTest.config$accessor(EventsByTagMigrationTest.scala:31)
[info]   at akka.persistence.jdbc.query.EventsByTagMigrationTest.withRollingUpdateActorSystem(EventsByTagMigrationTest.scala:161)
[info]   at akka.persistence.jdbc.query.EventsByTagMigrationTest.testFun$proxy1$1(EventsByTagMigrationTest.scala:188)
[info]   at akka.persistence.jdbc.query.EventsByTagMigrationTest.$init$$$anonfun$1(EventsByTagMigrationTest.scala:171)

I will push the fix after so we can have proof of the bug (and the bugfix)

jtjeferreira commented 5 months ago

Thanks for approving the workflows. There are 2 issues:

I haven't changed the workflows to use scala3. Any tip where I should do that? .github/workflows/checks.yml? Any examples from other akka projects I could follow?
Mima warnings in 2.13 on JdbcBackend#DatabaseDef vs JdbcBackend#JdbcDatabaseDef which changed in slick 3.5.0. Can I add a mima filter?

johanandren commented 5 months ago

I pushed a commit adding Test/compile on Scala 3 to CI PR validation now. There are some related failures to look into - methods that end up with the same signature after erasure.

jtjeferreira commented 5 months ago

I pushed a commit adding Test/compile on Scala 3 to CI PR validation now.

Thanks

There are some related failures to look into - methods that end up with the same signature after erasure.

I have seen that error locally. If I comment the @ApiMayChange it compiles. If I add it again and compile incrementally it works... I guess this is another compiler bug?

johanandren commented 5 months ago

If I comment the @ApiMayChange it compiles. If I add it again and compile incrementally it works... I guess this is another compiler bug?

Sounds like that, removing that annotation and adding Scaladoc saying it "API may change" is fine IMO

jtjeferreira commented 5 months ago

If I comment the @ApiMayChange it compiles. If I add it again and compile incrementally it works... I guess this is another compiler bug?

Sounds like that, removing that annotation and adding Scaladoc saying it "API may change" is fine IMO

Meanwhile, I found a workaround (see 45bbe4a).

I think I addressed all your feedback and also fixed mima after e6422a4

johanandren commented 5 months ago

Even better 👍

jtjeferreira commented 5 months ago

Regarding the failure in H2ScalaCurrentEventsByTagTest#should complete without any gaps in case events are being persisted when the query is executed I was also facing the problem locally, but I was hoping it was due to some slowness in my machine.

I fixed it with 7199f96, but I wonder if this is a problem with slick 3.5.0...

jtjeferreira commented 4 months ago

LGTM, thanks for the good work @jtjeferreira !

Thanks. Looking forward for a release

jtjeferreira commented 4 months ago

Looks like there is a problem in docs https://github.com/akka/akka-persistence-jdbc/actions/runs/8201918926/job/22431619062:

[error] missing argument for option -skip-packages

akka / akka-persistence-jdbc

Scala 3 support #775