search

haskell-bitcoin / bitcoin

Bitcoin Library for Haskell
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Feature/dependency pruning #27

Closed ProofOfKeags closed 2 years ago

ProofOfKeags commented 2 years ago

Closes #17 and #19

GambolingPangolin commented 2 years ago

Ok, the test suite actually should depend on aeson since there are a bunch of test vectors we parse out of json documents.

ProofOfKeags commented 2 years ago

ah yep. I need to add that to my workflow.

ProofOfKeags commented 2 years ago

OK So this now checks out. Lotta work to maintain the json stuff. Does it actually buy us anything or can we move to make the test vectors into in-haskell values?

ProofOfKeags commented 2 years ago

This PR should be squashed on merge.

ProofOfKeags commented 2 years ago

Is there some way to simplify the process of parsing test vectors out of JSON encoded sources? Ideally the JSON code would be totally minimal and purpose built to deal with the test vectors. So we would not need e.g. NetBox and JsonBox.

Probably. However, this change was primarily concerned with removing aeson and mtl from the main library code. I didn't really audit whether the code itself was "optimal". Personally I don't like the fact that we are giving typeclass instances to ToJSON and FromJSON to these types solely for the test vectors, as there is no guarantee that there is a canonical representation type here. I'd be fine with handrolling a Parser a for these types and using parseWith on the actual reading.

Keep in mind that a future PR will be moving from QuickCheck to hedgehog though so I'd rather not spend too much effort on this code in this particular patch. Happy to revamp the test code once the hedgehog patch is finished.

GambolingPangolin commented 2 years ago

All right. I don't mind going back for a cleanup pass. The instances don't bother me much since they are confined to the test suite.

ProofOfKeags commented 2 years ago

Actually I can revamp this code during the hedgehog patch.