-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: Add expect messages instead of unwraps #69
Conversation
Signed-off-by: Vaibhav Rabber <[email protected]>
src/client.rs
Outdated
basin_zone: basin_endpoint.map(|b| b.parse().unwrap()), | ||
cell: cell_endpoint | ||
.parse() | ||
.expect("should be a valid cell endpoint"), |
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.
preference for more affirmative expectations, e.g. valid cell endpoint
, that ideally mention why it is reasonable to have that expectation, e.g. previously validated cell endpoint
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.
updated
Signed-off-by: Vaibhav Rabber <[email protected]>
Signed-off-by: Vaibhav Rabber <[email protected]>
src/client.rs
Outdated
.user_agent(config.user_agent.clone()) | ||
.unwrap() | ||
.expect("previously validated user agent") |
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.
the reason here is that it is literally a HeaderValue
right?
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.
yup!
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.
how would you prefer specifying this?
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.
changed it to converting HeaderValue into HeaderValue
, feels much more clear
No description provided.