codingwell / scala-guice

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

Update to Scala 2.13 #98

Closed cacoco closed 2 years ago

cacoco commented 3 years ago

Problem

In order to prepare for the transition to Scala 3, it is helpful to move to support Scala 2.13.6 which is compatible with Scala 3.

Solution

Update the default Scala version of the project to Scala 2.13.6. We maintain all the previous cross compilation versions (bumping to Scala 2.13.6 here as well) so this should not have a material impact.

In order to support the lower versions we tee-code into version compatibility directories with the idea that at some point the src/main/scala-2.12- source directory will be dropped and the src/main/scala-2.13+ directory can be folded back into src/main/scala. This has no impact on library users only on maintainers and contributors.

Code which uses 2.13+ features is in src/main/scala-2.13+ and code which doesn't work with 2.13 is in src/main/scala-2.12-. The majority of the code is agnostic and remains in src/main/scala with changes made in some cases to make the code agnostic to Scala version (removal of Proxy usage in BindingProxies, removal of .mapValues usage in ScalaMapBinderSpec).

Lastly, a bit of clean up:

dotordogh commented 3 years ago

Have you tried running this with Scala 3? Because it still uses TypeTags, I suspect it won't compile. Are there plans to change the implementations to move away from TypeTags?

tsuckow commented 3 years ago

I haven't had a chance to look at this. I'm in a legal holding pattern with my employer about maintaining this library, hoping to get the i's dotted this month and look closer at it then. Pull requests are the best way to speed up that process once I'm unblocked.

cacoco commented 3 years ago

@dotordogh this was just to move to Scala 2.13. The Scala 3 and issue with TypeTag usage is a different question (and FWIW, this was done before we realized that problem). With that, this was meant to be a move to start segmenting the codebase such that we could potentially introduce Scala version-specific things. The TypeTag issue does likely mean some wholesale changes are going to be necessary and we can possibly introduce those changes in a scala-3 directory (maybe) but yeah this code wasn't necessarily meant to compile with Scala 3.

cacoco commented 3 years ago

@tsuckow we (Twitter) may be interested in taking over maintenance of this library if that helps at all via our team that also maintains the Finatra library where this is used. We're happy to have that discussion and talk through what it would mean.

tsuckow commented 2 years ago

My apologies for how long this took to merge. Shortly after you submitted this life got a little crazy and led to changing jobs. I thought it would be resolved the end of July and while the process wasn't complicated it took much longer than I was lead to believe it would. I was being extra cautious since my new employer also uses this library and didn't want any conflict of interest but in the end I got more of a shrug and sent on my way since I don't work on that code.

I am hesitant to turn the library over to a company simply because if they lose interest it will be stuck in limbo land forever.

Typetags does indeed appear to be an issue, it may be possible to use macros to replace them. I am no macro expert.

5.0.2 Should land on Maven Central Shortly. I don't think there are any significant changes other than the tail recursion hints.

For those interested in statistics, take these with a lot of salt as companies like to download libraries way more than they need to in continuous integration.

All image

2.11 image

2.12 image

2.13 image

cacoco commented 2 years ago

@tsuckow thanks for the details.

For this:

I am hesitant to turn the library over to a company simply because if they lose interest it will be stuck in limbo land forever.

I can understand that and FWIW this library is depended on by our Finatra project (which itself is used by thousands of microservices and applications inside Twitter alone) and honestly, we'd like to see this project be well-maintained and healthy as it directly benefits our Finatra project. But understand the hesitation there and we would need to have more internal deliberations anyway. Thanks!