Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Lwt_result #247

Merged
merged 5 commits into from Jun 21, 2016
Merged

add Lwt_result #247

merged 5 commits into from Jun 21, 2016

Conversation

c-cube
Copy link
Collaborator

@c-cube c-cube commented May 17, 2016

See #226 and #242
This is very basic, I can expand as required by nitpickers ;)

@hcarty
Copy link
Contributor

hcarty commented May 17, 2016

We should have Lwt.return_ok and Lwt.return_error, similar to Lwt.return_some.


val return : 'a -> ('a, _) t

val fail : 'b -> (_, 'b) t
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried this name may be a little confusing when seen with Lwt.fail. Would error or maybe return_error be clearer?

@c-cube
Copy link
Collaborator Author

c-cube commented May 17, 2016

I'm considering adding something like

val get_exn : ('a, exn) t -> 'a Lwt.t

to go with catch : 'a Lwt.t -> ('a, exn) t.
Any opinion?

@hcarty
Copy link
Contributor

hcarty commented May 17, 2016

get_exn 👍

@c-cube
Copy link
Collaborator Author

c-cube commented May 17, 2016

@Drup proposes to deprecate Lwt.make_value and Lwt.make_error. I have no opinion on that, what do you decide, @aantron?

@aantron
Copy link
Collaborator

aantron commented May 17, 2016

I think we should deprecate them, in agreement with @Drup.

@aantron aantron merged commit 2bf773a into ocsigen:master Jun 21, 2016
@aantron
Copy link
Collaborator

aantron commented Jun 21, 2016

Thanks @c-cube, @hcarty.

As agreed, we can make modifications in follow-on PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants