search

aantron / bisect_ppx

Code coverage for OCaml and ReScript
http://aantron.github.io/bisect_ppx/demo/
MIT License
301 stars 60 forks source link

Convert error propagation to monadic style for Ppxlib 0.28, 0.29 compatibility #413

Closed aantron closed 1 year ago

aantron commented 1 year ago

@pitag-ha, now that Bisect passes errors around using the With_errors monad, is it worth converting Bisect to also report errors using it? I assume the answer is yes.

For example, here and in other places:

https://github.com/aantron/bisect_ppx/blob/df530ff6f721719b0083fb6a8028ec5f16afff13/src/ppx/instrument.ml#L78-L79

Fixes #410.

pitag-ha commented 1 year ago

Hi @aantron,

Sorry for missing this!

About mondically propagating all errors in order to later embed them, including the ones that are currently raised: Yes, that would indeed be great! Btw, in case you'd like to read about embedding errors vs raising errors just out of interest: we now have a section about that in our manual (it doesn't cover the With_error style, though).

Also, btw, about your change: I don't understand the test file changes, but the instrument.ml changes look good.

pitag-ha commented 1 year ago

Hello @aantron,

Let me know if you have more questions about this. We're now in the OCaml 5.1.0 support phase. If bisect_ppx can get compatible with the latest ppxlib versions, it will also become compatible with OCaml 5.1.0; and it will form part of the next PPX universe and therefore will be taken into account wrt future breaking changes and patches.

aantron commented 1 year ago

Hi @aantron,

Sorry for missing this!

About mondically propagating all errors in order to later embed them, including the ones that are currently raised: Yes, that would indeed be great! Btw, in case you'd like to read about embedding errors vs raising errors just out of interest: we now have a section about that in our manual (it doesn't cover the With_error style, though).

Also, btw, about your change: I don't understand the test file changes, but the instrument.ml changes look good.

The response at this link is 404. I'm merging this as it is -- we can change the error generation later.

pitag-ha commented 1 year ago

The response at this link is 404.

Oh, there seems to be a problem with ocaml-doc-ci generating the documentation of our last release. Let me give the link for a fixed version as opposed to latest: https://ocaml.org/p/ppxlib/0.29.1/doc/good-practices.html#handling_errors

I'm merging this as it is -- we can change the error generation later.

Sounds good. Thanks!