Patrick Steinhardt <ps@xxxxxx> writes: >> > Is there a reason why we don't use `parse_options()` here? It might make >> > this tool easier to use and extend going forward, and we wouldn't have >> > to care about invalid arguments. Right now, we silently accept a >> > non-integer argument and do the wrong thing. >> >> I think that `parse_options()` would be overkill here because: >> >> - This is a _mandatory_ argument, not an optional one. >> >> - The required data type is `uint32_t`, and `parse_options()` has no >> support for that. > > Support for that has been merged just this week via 2bc5414c411 (Merge > branch 'ps/parse-options-integers', 2025-04-24). > >> But you do have a good point in that we may want to validate the data type >> (even if technically, this is not a user-facing program, it's a test >> helper that is used under tight control by Git's own test suite). >> >> Consequently, I would suggest this fixup instead: > > But in any case, I'd be equally fine with your suggestion. Yeah, I think we clearly showed our "it's just test helper, whose callers are supposed to know what they are doing" attitude, but with proper helpers, it is not too much additional effort to do the right thing. Thanks, both.
