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

Try turning teleproto into a wrapper around chimney #321

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

urmaul
Copy link

@urmaul urmaul commented Apr 18, 2024

Has the version number been increased?

  • Yes! (or not but it was intended to not increase it)

@urmaul urmaul changed the title Add Chimney dependency with a goal of turning teleproto into a wrapper around chimney Try turning teleproto into a wrapper around chimney Apr 18, 2024
@urmaul
Copy link
Author

urmaul commented Apr 19, 2024

The idea was to use chimney macros instead of teleproto macros. Calling Transformer.derive is easy but it works only when chimney can derive all recursive transformers. If we override some conversions by providing implicit writers, chimney doesn't care. It needs transformers.
If we don't find a way to make chimney use writer overrides, we'll have to convert them into transformers in every repository. That's manageable but not a drop-in replacement.
I tried converting stuff. I managed to turn chimney transformers into teleproto writers but not vice versa (error covariant type P occurs in contravariant position...).
I'll look more into it but so far by best idea is to teach teleproto use chimney transformers as overrides when they are available. Then we can convert our app-specific writers one by one and when all of them are converted... Dunno.

@urmaul
Copy link
Author

urmaul commented Apr 19, 2024

To make it nicer we need something like:

trait Writer[M, P] extends Transformer[M, P]

but in this case you can't have covariant/contravariant type parameters for Writer. And in this case, VersionedModelWriter doesn't compile because ot has a map of writers to different types...

@MateuszKubuszok
Copy link

I'd say if you want to offload things to Chimney, while keep an easy way to add your own staff, it would be easier to just use the macros directly:

This way:

  • there would be no overhead for converting back and forth
  • your API would NOT rely on Transformers/PartialTransformers
  • you could inject your custom rules without fighting with variance, implicit priority, etc

It's pretty much doable, although it rely on undocumented internal APIs, so feel free to open a discussion if you want to try this approach.

@urmaul
Copy link
Author

urmaul commented Jun 21, 2024

I'd say if you want to offload things to Chimney, while keep an easy way to add your own staff, it would be easier to just use the macros directly:

Sounds like it could solve the problems I have with it. I'll look into it.

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.

None yet

2 participants