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

Get Lwt_result closer to Stdlib.Result #927

Merged
merged 3 commits into from
Feb 10, 2022
Merged

Conversation

MisterDA
Copy link
Contributor

@MisterDA MisterDA commented Feb 1, 2022

Lwt_result has the double job of mapping fulfilled promises to Ok and rejected (with an exception) to Error, but also being useful to promises returning results. I had a little piece of code that was missing the iter and error functions, so I added them.

Add Lwt_result, Lwt_result.iter, Lwt_result.iter_error.
Alias Lwt_result.map_err and Lwt_result.bind_lwt_err to Lwt_result.map_error and Lwt_result.bind_lwt_error for consistency with Stdlib. Maybe that part with the deprecation is too much.

Took the liberty of adding @since 5.6.0 to the new functions but that may not be the proper version number.

Alias Lwt_result.map_err and Lwt_result.bind_lwt_err to
Lwt_result.map_error and Lwt_result.bind_lwt_error for consistency
with Stdlib.
For consistency with Stdlib.
Lwt.map
(function
| Error e -> Error (f e)
| Ok x -> Ok x)
e
let map_err f e = map_error f e
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we mark this as deprecated as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the deprecation warning in the interface sufficient?

Copy link
Member

Choose a reason for hiding this comment

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

Oops! I missed that they were at the end of the file!

Lwt.bind e
(function
| Error e -> Lwt.bind (f e) fail
| Ok x -> return x)
let bind_lwt_err e f = bind_lwt_error e f
Copy link
Member

Choose a reason for hiding this comment

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

Same

@smorimoto smorimoto merged commit 5c333d0 into ocsigen:master Feb 10, 2022
@MisterDA MisterDA deleted the lwt_result branch March 18, 2022 14:08
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.

2 participants