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

Implemented is OpaqueType method #333

Open
wants to merge 1 commit into
base: scala3
Choose a base branch
from
Open

Implemented is OpaqueType method #333

wants to merge 1 commit into from

Conversation

Pask423
Copy link
Contributor

@Pask423 Pask423 commented Aug 11, 2021

Solves opaque types task in #296

@ghost ghost requested a review from adamw August 12, 2021 04:52
@adamw
Copy link
Member

adamw commented Aug 12, 2021

We definitely need a test :)

@KacperFKorban
Copy link
Collaborator

The problem with opaque types is actually a bit more complex. Magnolia for Scala 3 is pretty much just a wrapper for Mirrors (at least for now) and Mirrors aren't generated for opaque types. And I'm pretty sure that it was a design decision. This means that magnolia cannot derive instances for opaque types.
These changes seem to work as intended, but at the current state they will always result in isOpaque = false.
They will be very useful once #321 works, but unfortunately, I haven't had time to work on it lately.
Not sure if we should hold this PR or merge it anyway with the intent for it to work in the future. @adamw?

@adamw
Copy link
Member

adamw commented Aug 12, 2021

I think it's better to hold off with this PR, as it would give the false impression that this feature works, while it doesn't

We should probably concentrate on macrolia then :)

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

3 participants