Re: [PATCH] builtin/receive-pack.c: use parse_options API

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Sidhant Sharma <tigerkid001@gmail.com> writes:

>>> +	if (argc > 1)
>>> +		usage_msg_opt(_("Too many arguments."), receive_pack_usage, options);
>>> +	if (argc == 0)
>>> +		usage_msg_opt(_("You must specify a directory."), receive_pack_usage, options);
>> Before that, the loop was ensuring that service_dir was assigned once
>> and only once, and now you check that you have one non-option arg and
>> assign it unconditionally:
>>
>>> +	service_dir = argv[0];
>> ... so isn't this "if" dead code:
>>
>>>  	if (!service_dir)
>>> -		usage(receive_pack_usage);
>>> +		usage_with_options(receive_pack_usage, options);
>> ?
>>
>>
> Yes, I just realized that is dead code (sorry). Removing the 'if'
> statement would correct that?

Yes.

> Also, is the unconditional assignment to service_dir correct in this
> case, or should some other test condition be added?

Since usage_msg_opt is NORETURN, it's OK: if you reach this point, you
know that argv[0] contains something.

> Another thing I'd like to ask is when I prepare the next patch, should
> it be sent as reply in this thread, or as a new thread?

No strict rule on that, but I usually use --in-reply-to on the root of
the thread for previous iteration. If you don't, include a link (e.g.
gmane) to the previous iteration in the cover-letter.

format-patch has a -v2 option to let you get [PATCH v2 ...]
automatically.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/