On 5/27/24 12:55, Peter Krempa wrote:
> On Mon, May 27, 2024 at 11:18:50 +0200, Michal Privoznik wrote:
>> We currently require full argument specification:
>>
>>   virt-admin daemon-timeout --timeout X
>>
>> Well, the '--timeout' feels a bit redundant. Turn the argument
>> into a positional so that the following works too:
>>
>>   virt-admin daemon-timeout X
>>
>> Signed-off-by: Michal Privoznik <[email protected]>
>> ---
>>  docs/manpages/virt-admin.rst | 2 +-
>>  tools/virt-admin.c           | 1 +
>>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> It was a bit of a deliberate decision, because any positional argument
> can't be made optional and must be kept in order. Historically also the
> parser did weird things for stuff that wasn't really considered
> positional but was parsed so regardless.
> 
> In this case though ... I can't really see another option being added or
> timeout becoming optional especially since '0' is used as way to disable
> the timeout.

This was exactly my internal reasoning when writing this patch because I
too dislike positional arguments.

> 
> So your justification here makes sense, it's just that I really don't
> like positional arguments.

Since our docs are fixed by previous patch, this one is just a
'shortcut' and strictly speaking it's not needed. I can drop it, I don't
mind.

> 
> Reviewed-by: Peter Krempa <[email protected]>
> 

Michal

Reply via email to