-
Notifications
You must be signed in to change notification settings - Fork 28
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
Allow 'None' error type #78
base: main
Are you sure you want to change the base?
Conversation
for code paths that are like: ``` let error = if something() { ResponseError::None } else { ResponseError::InvalidMsg }; ```
This is failing because |
I think we should continue to represent no error as I do see that the logic you need is a bit verbose to express currently. let error = if auth_succeeded() {
None
} else {
Some(ResponseError::InvalidMsg)
};
ResponseBuilder::default().with_error_code(error.map(|x| x.code()).unwrap_or_default()); But maybe we could change the pub fn with_error_code(mut self, value: Option<ResponseError>) -> Self { Then we could use it like: let error = if auth_succeeded() {
None
} else {
Some(ResponseError::InvalidMsg)
};
ResponseBuilder::default().with_error_code(error); Does that sufficiently improve the ergonomics? |
I agree that keeping the ergonomics for receiving is important. |
i think adding the use of |
This could potentially be changed to return an error in the case of unknown rather than having the catch all enum branch. In either case, the current method is now infallible so the try naming and option return type doesn't make sense. |
this allows for code paths that are like: