rgrinberg / onanomsg

nanomsg bindings for ocaml
Do What The F*ck You Want To Public License
38 stars 9 forks source link

Added initial async support, refactoring (use CCError.t instead of ex… #7

Closed vbmithr closed 9 years ago

vbmithr commented 9 years ago

…ceptions).

For review. I'm not sure using error monads was a good move. Overall I tend to prefer an error monad to an exception nowadays, but I'm a bit worried to make this interact with async.

Basically I made those change since I'm working on an async backend…

rgrinberg commented 9 years ago

Looks good but it's a little strange to use CCError.t in conjunction with async and not core's own error type.

Of course the alternative is writing functors so I can understand.

vbmithr commented 9 years ago

There is a bug in this code with respect to Async handling of file descriptors. I'm gonna push a fix. And also write more tests.

vbmithr commented 9 years ago

About CCError, the advantage of it is that is uses polymorphic variants, which can easily coexist with… whitequark's code by example, as opposed to normal variants.

rgrinberg commented 9 years ago

Yes. Although in the future, core's error type will be in pervasives.

vbmithr commented 9 years ago

On 02/06/2015 18:15, Rudi Grinberg wrote:

Yes, although in the future, core's error type will be in pervasives.

Would be awesome! I'm sucks to have to work with different versions of the same thing.

Vincent