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

Instances for types from small packages #432

Closed
phadej opened this issue Jun 15, 2016 · 5 comments
Closed

Instances for types from small packages #432

phadej opened this issue Jun 15, 2016 · 5 comments

Comments

@phadej
Copy link
Collaborator

phadej commented Jun 15, 2016

I realised that I might need ToJSON for These.

For now I'll go with orphan instance in my application, but in longer run, where to put those instances?

@bergmark
Copy link
Collaborator

What are the alternatives?

Add them ...

  • to aeson - Doesn't scale
  • to otherpackage - up to each maintainer
  • under a Cabal flag in aeson or otherpackage - Ekmett does this for some packages, where the default is to include the dependency. But effectively this means that the dependency is incurred.
  • to an aeson-otherpackage package. Example: https://hackage.haskell.org/package/uuid-aeson

Unless there are options I didn't mention, only the last alternative seems reasonable to me.

At Silk we have a custom prelude which imports all orphan instances to avoid not-imported problems, but a lot of packages may not want a custom prelude.

@andrewthad
Copy link
Contributor

I agree with @bergmark. Picking up a dependency on these just to provide the instance doesn't seem worth it. It looks like these depends on data-default-class and hashable, so maybe the author would be interested in incurring an aeson dependency.

@phadej
Copy link
Collaborator Author

phadej commented Jun 23, 2016

@andrewthad, sorry for being unclear. these is mere a single example. I guess there should be some consistency about those, probably explicitöy mentioned in CONTRIBUTING.md or such.

http://hackage.haskell.org/package/quickcheck-instances clearly mentions

The goal is to supply QuickCheck instances for types provided by the Haskell Platform.

IMHO aeson should as well, so that e.g. contravariant question won't be asked later again

@bergmark
Copy link
Collaborator

If someone creates such a package we can make a note of it somewhere in aeson marking "blessed" external instances. If we decide to add any of those to aeson we'll sync with that package to prevent breakage.

@phadej
Copy link
Collaborator Author

phadej commented Apr 9, 2020

The discussion about aeson These instances arose again.

What's your opinion nowadays, @bergmark. If I strip these to bare minimum, probably depending only assoc and hashable (+ GHC boot libs), would it be possible that ToJSON (These a b) etc to be moved to aeson?

Over past years my opinion become that These is a quite core type, and spreading it around would be easier if the these package become as small as possible, but I don't want make these-aeson just for few instances - I don't think it would make anything better, only increase the maintenance load.

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

No branches or pull requests

3 participants