Skip to content

Conversation

kvc0
Copy link

@kvc0 kvc0 commented Jan 13, 2023

unwrap() gives no clues where to look when a bug arises. This change
replaces common tdigest unwrap() calls with expect("message").

This does the same thing as unwrap(), it just provides more context
for users to report with bugs.

Fwiw, I find that explaining an expectation in rust can sometimes
show me that the expectation is unreasonable. I can't come up with
good explanations for some of these, but I gave an effort to at
least provide a first-approximation of what the expectation at work
is. Any refinements to what these expectations are actually trying
to assert would be welcome!

It would be nice to be able to provide more information for #680

I've only got a couple years of experience with Rust, but I'd strongly
recommend taking a "no unwrap() outside of tests" policy!

unwrap() gives no clues where to look when a bug arises. This change
replaces common tdigest unwrap() calls with expect("message").

This does the same thing as unwrap(), it just provides more context
for users to report with bugs.

Fwiw, I find that explaining an expectation in rust can sometimes
show me that the expectation is unreasonable. I can't come up with
good explanations for some of these, but I gave an effort to at
least provide a first-approximation of what the expectation at work
is. Any refinements to what these expectations are actually trying
to assert would be welcome!
@CLAassistant
Copy link

CLAassistant commented Jan 13, 2023

CLA assistant check
All committers have signed the CLA.

@epgts epgts self-assigned this Jan 17, 2023
@syvb syvb unassigned epgts Jun 19, 2023
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.

3 participants