-
Notifications
You must be signed in to change notification settings - Fork 150
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
ποΈ build(play-json): upgrade to Play JSON 3.0 #380
Conversation
Also I didn't find any changelog, this change is worth mentioning with maybe an extra word because downstream users will have to choose in their dependency tree if they keep using |
@lloydmeta one last thing, I was not able to run tests locally on my machine when running following commands:
It fails with some compilation errors that I couldn't understand what to do to avoid them:
What am I missing? |
Codecov Report
β Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #380 +/- ##
=======================================
Coverage 85.52% 85.52%
=======================================
Files 65 65
Lines 518 518
Branches 31 31
=======================================
Hits 443 443
Misses 75 75 |
I think what possibly complicates things (more!) is that the enumeratum-play module has a dependency on enumeratum-play-json. Line 357 in dba1630
To be honest, at this point, I would not be sad if we dropped all support going forward for Play < 3 in favour of just Play 3+ . Having read the doc on the Play website, moving ahead with Pekko-based Play and encouraging everyone to do so feels like something that makes sense. So, I would say, if it simplifies a lot of things to just do this in order to move enumeratum-play to Play 3, let's just drop support for older versions. That said, if you're (or anyone reading this) strongly opinionated against this, please shout.
I think you might be missing passing Line 141 in dba1630
I wonder if it can be made optional. |
Makes sense to me as well. It's what Tapir did. And I guess that if there's demand for Play < 3 compatibility, it could still be made available with a dedicated module. But I would tend to avoid it in the 1st place. Thanks for the info, I'll try to work on Play 3 today or tomorrow. |
I believe this can be closed since #381 has been merged. |
Bump play-json dependency in enumeratum-play-json module to 3.0.
Play-JSON 3.0 is strictly the same in terms of code compared to 2.10.1 (see the diff: playframework/play-json@2.10.1...3.0.0). The major change is due to the groupId renaming from
com.typesafe.play
toorg.playframework
.Note that I didn't work on upgrading to Play 3.0 in enumeratum-play for now as there's a bit more work but I'd happy to give it a try in the coming days.
I'm not entirely sure how to go about it though, as this may force downstream users to upgrade (mainly because of the underlying change from Akka to Pekko in Play). I would need to check if enumeratum can be compatible with both Play 2 and Play 3.
I guess I'll provide another PR and then we can discuss and maybe test compatibility on real projects (I have some at my company that can be good candidates).