Attention is currently required from: Evan Benn.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71973 )
Change subject: flashrom_tester: Rewrite IOOpts to support more operations ......................................................................
Patch Set 7:
(1 comment)
File util/flashrom_tester/flashrom/src/cmd.rs:
https://review.coreboot.org/c/flashrom/+/71973/comment/1513c372_1e577b4f PS6, Line 73: Option
your 3 cases are not quite right, these are the two modes flashrom accepts: […]
What you've got in PS7 seems reasonable, but I'd like some inline documentation on the fields because it's a little confusing when there are two `Path`s but one is optional.
I'd probably just convert the comments to docstrings, then you can write more detail:
``` /// File is the size of the full chip, limited to a single named region. /// /// The required path is the file to use, and the optional path is a layout file /// specifying how to locate regions (if unspecified, flashrom will attempt /// to discover the layout itself). FullFileRegion(&'a str, &'a Path, Option<&'a Path>), /// File is the size of the single named region only. /// /// The required path is the file to use, and the optional path is a layout file /// specifying how to locate regions (if unspecified, flashrom will attempt /// to discover the layout itself). RegionFileRegion(&'a str, &'a Path, Option<&'a Path>), ```