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

[scala3] Add test for serializable generated type-class #499

Open
wants to merge 3 commits into
base: scala3
Choose a base branch
from

Conversation

RustedBones
Copy link
Contributor

Scala3 version of magnolia does not generate serializable typeclass when defined inside a non-serializable outer, unlike the scala 2 version. Tested in #498.

It looks like the summonInline[Typeclass[p]] lambda in the CallByNeed takes a reference on the outer obejct.

@RustedBones RustedBones changed the title Typeclass serialization tests [scala3] Add test for serializable generated type-class Nov 13, 2023
@RustedBones
Copy link
Contributor Author

@adamw since you reviewed #498

This new test is only relevant for JVM, and fails for JS and native.
The project layout for magnolia looks non standard. I don't know how to add JVM only test.

Would migrating to sbt-corssproject be considered ? I can submit a PR for that.

@adamw
Copy link
Member

adamw commented Jan 7, 2024

@RustedBones our projects use https://github.com/sbt/sbt-projectmatrix, in my experience this is the only sane way to maintain projects with both simple & complex cross-platform setups. I think adding the test in scalajvm would do the trick (as opposed to scala), see e.g. here.

@RustedBones
Copy link
Contributor Author

Ok, let's stick with sbt-projectmatrix.
At the moment, there isn't a scala folder. The project defines fake project root and overrides the scalaSource. (btw, this setup does not work well with intelliJ and requires to manually add source to get indexer working).
What about standardizing the project layout like this and create a scalajvm source folder in the test project ?

@RustedBones RustedBones marked this pull request as ready for review January 10, 2024 15:11
@RustedBones
Copy link
Contributor Author

Any take on this one?
There is probably a trick to avoid this but I couldn't find any other alternative

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