-
Notifications
You must be signed in to change notification settings - Fork 122
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
Refresh: Update Result<T, B>
to be Result<(T,B), Error<B>
#295
base: master
Are you sure you want to change the base?
Conversation
@ileixe would you care to review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pushing this forward. We're waiting for a while this :)
I usually prefer qualifying Error
and Result
, but on the other hand I thought Buf prefix worth its own as it's a bit different from plain Result. (We're using BufResult and Result in the same place). Let me leave it to someone else.
@ileixe Does this address your comment? |
@ollie-etl |
@ileixe That makes sense - done |
This PR is a update of #267, which seems to have gone stale, due to inactivity on in this repo.
Changes the return type to be a Result of tuples, instead of a tuple containing a result and a buffer. This allows this crate to work in a more ergenomic fashion with
?
operatorIn addition to updating the original PR, the following changes are made in this one, not in the original
BufError
renamedError
. This follows the more usual convention of using crate prefixes to qualify error types, rather than the type name itself.BufResult
renamedResult
. This is the same rationalmap_buf
helper function implrmented with a sealed trait, to give method calling convention, rather than qualified callscloses #178