Skip to content

Conversation

ethul
Copy link

@ethul ethul commented Feb 17, 2017

Would you consider accepting this PR to add a Generic instance for HugeInt?

Thanks!

@chexxor
Copy link

chexxor commented Feb 17, 2017

Should be OK, as we added Generic instances for other types in this lib - #6

@garyb
Copy link
Member

garyb commented Feb 17, 2017

I think maybe this should just be derive rather than derive newtype... derive newtype for Generic seems suspect to me since the derived values will be indistinguishable from the generics for the underlying type, so I don't know if they'll behave right when trying to examine/reconstruct something. I could be wrong though, if you've tried it out and it works as expected, then I guess it's fine!

@ethul
Copy link
Author

ethul commented Feb 17, 2017

I think you bring up a good point. Let me verify by looking at both derive by itself and derive newtype. I will provide an update on this later today or tomorrow.

@ethul
Copy link
Author

ethul commented Feb 17, 2017

You're right that with the newtype we get tag: "Data.HugeNum.HugeNum" instead of tag: "Data.HugeInt.HugeInt". The Generic instance with the derive newtype is working, but perhaps in the spirit of purescript-precise the second choice is a more fitting implementation. I will make the change!

@garyb
Copy link
Member

garyb commented Feb 17, 2017

Thanks!

@garyb garyb merged commit dbef11b into purescript-contrib:master Feb 17, 2017
@ethul
Copy link
Author

ethul commented Feb 17, 2017 via email

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

Successfully merging this pull request may close these issues.

3 participants