Skip to content

Commit 0f39350

Browse files
roypatShadowCurse
authored andcommitted
doc: Add CONTRIBUTING guideline about .unwrap() usage
Clarifies that we want to avoid unwraping in favor of propagating errors. Closes #808 Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
1 parent db24c11 commit 0f39350

File tree

1 file changed

+27
-0
lines changed

1 file changed

+27
-0
lines changed

CONTRIBUTING.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,33 @@ Your contribution needs to meet the following standards:
119119
}
120120
```
121121

122+
- Avoid using `Option::unwrap`/`Result::unwrap`. Prefer propagating errors
123+
instead of aborting execution, or using `Option::expect`/`Result::except` if
124+
no alternative exists. Leave a comment explaining why the code will not panic
125+
in practice. Often, `unwrap`s are used because a previous check ensures they
126+
are safe, e.g.
127+
128+
```rs
129+
let my_value: u32 = ...;
130+
if my_value <= u16::MAX {
131+
Ok(my_value.try_into::<u16>().unwrap())
132+
} else {
133+
Err(Error::Overflow)
134+
}
135+
```
136+
137+
These can often be rewritten using `.map`/`.map_err` or `match`/`if let`
138+
constructs such as
139+
140+
```rs
141+
my_value.try_into::<u16>()
142+
.map_err(|_| Error::Overflow)
143+
```
144+
145+
See also
146+
[this PR](https://github.com/firecracker-microvm/firecracker/pull/3557) for a
147+
lot of examples.
148+
122149
- Document your pull requests. Include the reasoning behind each change, and the
123150
testing done.
124151

0 commit comments

Comments
 (0)