Avoid using unwrap #184
Avoid using unwrap #184
Conversation
jack-signal
force-pushed
the
jack/no-unwrap
branch
2 times, most recently
from
b2d8b08
to
30066ec
Compare
|
|
||
| // prost documents the only possible encoding error is if there is insufficient | ||
| // space, which is not a problem when it is allowed to encode into a Vec | ||
| structure.encode(&mut result).expect("No encoding error"); |
There was a problem hiding this comment.
danburkert/prost#154 😭
There was a problem hiding this comment.
https://github.com/danburkert/prost/pull/378 stalled since October :(
| @@ -625,7 +632,7 @@ mod tests { | |||
| use rand::rngs::OsRng; | |||
| use rand::{CryptoRng, Rng}; | |||
|
|
|||
| fn create_signal_message<T>(csprng: &mut T) -> SignalMessage | |||
| fn create_signal_message<T>(csprng: &mut T) -> Result<SignalMessage> | |||
There was a problem hiding this comment.
This one doesn't really make sense to me. It may be a helper function for a test, but it actually cannot fail, and immediately panicking is valid. Can you use expect instead?
There was a problem hiding this comment.
Sure I guess. Reasoning was to avoid as many expects in src as possible since those need to be audited for panics, and having tests using unwrap/expect makes it harder to find ones that are in the protocol src proper.
There was a problem hiding this comment.
Eh, my opinion is that auditing is all well and good, but not at the cost of actual type signatures. I can see how being a test helper puts it on the edge, though.
Another option would be to go back to use unwrap in tests, since the Clippy warning keeps it out of the src files. Any of these three options are all right with me though.
jack-signal
force-pushed
the
jack/no-unwrap
branch
2 times, most recently
from
68f5945
to
84c7a58
Compare
jack-signal
force-pushed
the
jack/no-unwrap
branch
2 times, most recently
from
b77a689
to
1346cb6
Compare
jack-signal
force-pushed
the
jack/no-unwrap
branch
from
1346cb6
to
14060ae
Compare
|
@jrose-signal ready for another look |
Remove some unwrap from poksho and futures crates
jack-signal
force-pushed
the
jack/no-unwrap
branch
from
bae72b7
to
a56874e
Compare
If we must assert that a value is
Ok/Someuseexpectwith a string explaining why this must be so.Most of the changes here are in the tests, where of course
unwrapis mostly harmless. But updating there as well makes it easy to grep for any other uses.(This skips
unwrap_orbecause those don't panic)