Re: [PATCH] builtin/receive-pack.c: use parse_options API
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Mar 1, 2016, 17:57 UTC
- Message-ID
- <vpqio1613p1.fsf@anie.imag.fr>
- In-Reply-To
- <56D5D601.8030601@gmail.com>
Sidhant Sharma <tigerkid001@gmail.com> writes:
Show 19 quoted lines
>>> + 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/