-
Notifications
You must be signed in to change notification settings - Fork 115
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
Magnolia derivation for Debug typeclass #1371
Conversation
1f2cb8b
to
d9ae483
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is lovely to see. Could you please adjust a few things so that we can merge this?
project/BuildHelper.scala
Outdated
"com.softwaremill.magnolia1_3" %% "magnolia" % "1.3.7" | ||
case _ => | ||
"com.softwaremill.magnolia1_2" %% "magnolia" % "1.1.10" | ||
// libraryDependencies += compilerPlugin("org.typelevel" %% "kind-projector" % "0.13.3" cross CrossVersion.full) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commented code in build
@@ -0,0 +1,38 @@ | |||
package zio.debug.magnolia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package name should be zio.prelude.magnolia
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about adding an extension method on the Debug companion object, so that it also contains the derived
method, so that in Scala 3 we can do case class Child(name: String, age: Int) derives Debug
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whhhhhaou that exactly what I did not knew how to do !
Thx, will do it tonight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, I suppose then that I should merge magnolia and core module, right ?
Or AFAIU I run in cyclic dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cyclic dependencies
Nope. Have zio-prelude-magnolia
depend on zio-prelude
. zio-prelude
must not know about Magnolia or ``zio-prelude-magnolia`.
That's why we'll make it an extension method of the Debug
companion object. Not an actual method on Debug
companion object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:-/
Just pushed the simplest fix.
No idea how to add an extension to an existing companion object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok @sideeffffect I found :S
ci: 👌 @sideeffffect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you so much again! 🎉
You welcome.
Thanks for the review and tips.
I was not aware that extension could target a companion object.
Thx
Le jeu. 5 sept. 2024 à 21:52, Ondra Pelech ***@***.***> a
écrit :
… ***@***.**** approved this pull request.
Awesome, thank you so much again! 🎉
—
Reply to this email directly, view it on GitHub
<#1371 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC4NK7PJNUVLNV26SDBJBLZVCY67AVCNFSM6AAAAABNUYQPLCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOBTHE2TQMBTHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
closes #1076