Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

Add Data instance to Record #56

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

Conversation

locallycompact
Copy link
Contributor

I think this is right

@Dridus
Copy link
Contributor

Dridus commented Mar 28, 2022

@asariley @dfithian can one of you shepherd this PR? I assigned you both to review, but what I really mean is either one of you

@dfithian
Copy link
Collaborator

This seems reasonable, but I wonder if it should really be defined in the Vinyl library, since there's nothing specific to Composite about the code that was added. If you were to do that, we might need to do some more work to get it compiling with Vinyl 0.14, since that's not allowed by our version constraints.

In that case, you might just keep using your fork in the short term and we can work on supporting Vinyl 0.14.

@dfithian
Copy link
Collaborator

The latest ghc 9.2.2 branch supports vinyl 0.14, so if that was added it would get pulled in automatically. I think we could probably close this now?

@locallycompact
Copy link
Contributor Author

I think we will still need this bit

deriving instance (Typeable k, Data k, KnownSymbol s) => Data (s :-> k)

Let me just check I can correctly implement the rest for Rec f because at the minute it is Rec Data.Functor.Identity

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants