TomasMikula / libretto

Declarative concurrency and stream processing library for Scala
Mozilla Public License 2.0
199 stars 7 forks source link

Future proofing: Rewrite implicit to Scala 3 replacements #81

Closed MateuszKowalewski closed 1 year ago

MateuszKowalewski commented 1 year ago

Almost all implicits are gone:

Screenshot_20230629_184658

The rest needs special care. I'll open issues.

The PR is divided in two commits for review:

The commits only compile when applied together. So this is a WIP PR.

After review / discussion I'll rewrite it to one big single commit (with a better commit message).

MateuszKowalewski commented 1 year ago

Exactly!

Especially as I have a commit on top already waiting, rewriting all the imports to Scala 3 syntax… :smile:

MateuszKowalewski commented 1 year ago

Good job overall!

Thanks a lot!

It took a little bit longer than I've thought as there were some tricky parts to find, but all in all it was a good issue to look around the code.

MateuszKowalewski commented 1 year ago

I've done what we said (see the diff of the force push).

But I'm going to revert the removal of implicit classes in CoreLib. This breaks exactly the same as with the other implicit classes only that the extension methods / implicit class methods aren't called anywhere.

Looks like missed test coverage. (I'll open an issue).

TomasMikula commented 1 year ago

Thanks again for this!