search

ocaml-dune / csexp

Minimal support for Canonical S-expressions
MIT License
27 stars 7 forks source link

Do not use result module #3

Closed rgrinberg closed 4 years ago

rgrinberg commented 4 years ago

I'm not sure why it was included in the interface, but it seems like it shouldn't belong there.

Also, I've changed the result module to be used without changing the code. This makes it easier to embed csexp in places where result isn't present.

ghost commented 4 years ago

That definitely seems like a mistake.

ghost commented 4 years ago

Ah but wait, doesn't that mean the code is vulnerable to addition to the Result module now? I remember this was a problem for some packages in the past.

ghost commented 4 years ago

@rgrinberg I'll revert that part. For the copy in Dune that won't be an issue soon as we will bump the lower bound on OCaml to 4.08. In the meantime, we can simply add a sed operation in the update script.

ghost commented 4 years ago

I'm also making a bugfix release.