codingwell / scala-guice

Scala extensions for Google Guice
Apache License 2.0
341 stars 44 forks source link

Scala 3 #103

Closed russellremple closed 1 year ago

russellremple commented 1 year ago

Thought I'd take a swing at Scala 3?. This is my first PR here, so guidance is welcome.

The comment that "It seems it would probably require a macroized version of TypeConversions, assuming that is even possible." was exactly right. This PR introduces a new version of this component using the Scala 3 macro system. There are many parallels with the Scala 2 implementation, and they are structurally similar in terms of recursively deconstructing the type to build a TypeLiteral, but the details are quite different.

It and the other 7 main and 3 test source files under net.codingwell.scalaguice that relied on TypeTag were all moved to a scala-2 source directory without changes. Copies of these were created under a scala-3 source directory with the following small changes:

Other changes in support of this PR include:

There is a lot more code duplication here than I'd like, with the obvious downsides of that, but not sure how to avoid it. The upside is that your confidence that I have not messed up the Scala 2 artifacts should be very high.

tsuckow commented 1 year ago

It will probably be a couple weeks before I can get around to reviewing this, but I am excited.

tsuckow commented 1 year ago

Nice to see extends AnyVal doesn't seem to be needed.

Is the REstore NV copyright still appropriate in the Scala3 version of the type conversions? It would appear you completely rewrote that file.

As an aside I'm looking at updating my build VM I use to publish from. I'm hoping to get back to this around the 11th, unfortunately I am going to mostly be kept from my keyboard until then but did manage to sneak some time today to read through most of the changes.

russellremple commented 1 year ago

Nice to see extends AnyVal doesn't seem to be needed.

Is the REstore NV copyright still appropriate in the Scala3 version of the type conversions? It would appear you completely rewrote that file.

As an aside I'm looking at updating my build VM I use to publish from. I'm hoping to get back to this around the 11th, unfortunately I am going to mostly be kept from my keyboard until then but did manage to sneak some time today to read through most of the changes.

Thanks! You're right, that part is probably rewritten to the point the copyright isn't appropriate. I'll remove it.

I wasn't sure about your versioning/release strategy. Seems like you are trying to keep the versioning in sync with Guice versioning. If you want to keep the version number the same and just release a _3 artifact, that works for me. Or maybe you want a new patch version for all Scala versions, and bump the Guice version accordingly? I'm open to however you'd like it - just let me know on the 11th.

tsuckow commented 1 year ago

V5.1.1 should sync to Maven Central shortly.

Normally I would post the download statistics for the the various scala builds here, but SonaType seems to be b0rked and it throwing 500's ¯\_(ツ)_/¯.