search

byteverse / disjoint-containers

Other
6 stars 4 forks source link

Remove Aeson dependency #10

Closed j-hui closed 5 years ago

j-hui commented 5 years ago

Is it really necessary to Aeson as a dependency for this library?

As far as I can tell, the only time Aeson is really used is for testing, and for this type class instance declaration: https://github.com/andrewthad/disjoint-containers/blob/master/src/Data/DisjointMap.hs#L99

While this may be useful for very specific cases involving JSON parsing, it also attaches a bunch of dependencies that don't seem necessary for the functionality that this library provides.

Could the Aeson dependency be moved outside of the library?

chessai commented 5 years ago

I believe the JSON instances exist for actually encoding/decoding structures to/from JSON, and so are not used in this library. I'd rather not remove them, but we could put the dependency of aeson behing a flag? Eg you could compile with -f-aeson and it would not build the library against aeson or provide its instances.

@andrewthad what do you think?

chessai commented 5 years ago

Also, @j-hui would this flag-style approach work for you?

andrewthad commented 5 years ago

These instances were historically used by an older application. Since there aren't any active projects that use these instances, it should be fine to just remove them. In the event that I need them again, I could always split them into their own package, not as orphan instances but as plain functions. I'd take a PR that removed them.