Skip to content
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

Reintroduce automatic derivation of Get and Put #1350

Conversation

oyvindberg
Copy link
Contributor

@oyvindberg oyvindberg commented Jan 21, 2021

Followup from #1343 .

I made a proposal in softwaremill/magnolia#278 , and this implements it on the doobie side.

I fully expect some discussion about what it'll look like in the end, just opening this in a draft state this to show that it might be easy to fix.

… types with magnolia (requires magnolia snapshot)
@oyvindberg oyvindberg marked this pull request as draft January 21, 2021 23:35
@oyvindberg oyvindberg marked this pull request as ready for review January 22, 2021 17:47
@oyvindberg
Copy link
Contributor Author

This is a concrete proposal now that doesn't change anything in magnolia anymore. I'd still wait a bit on feedback on the PR I opened there as this could still be better solved on that side I think.

It's obviously not entirely optimal to define local macro in doobie, but it's only for scala 2, and it's not that complex I think.

An alternative to this solution is to resurrect the shapeless dependency and use that only to infer Get and `Put.


case class No1(i: Int, s: String)
object No2
case class Y private (x: String)
Copy link
Collaborator

@jatcwang jatcwang Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used? (and it shouldn't be possible, since constructor is private)

@tpolecat
Copy link
Member

tpolecat commented Mar 8, 2021

I'm losing confidence that this is the way to go. For now I'm going to revert #1343 and maybe we can revisit if it's fixed in Magnolia.

@tpolecat tpolecat closed this Mar 8, 2021
@tpolecat tpolecat mentioned this pull request Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants