From: Matthieu Moy <Matthieu.Moy@grenoble-inp•fr>
To: Sidhant Sharma <tigerkid001@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] builtin/receive-pack.c: use parse_options API
Date: Tue, 01 Mar 2016 18:57:30 +0100 [thread overview]
Message-ID: <vpqio1613p1.fsf@anie.imag.fr> (raw)
In-Reply-To: <56D5D601.8030601@gmail.com> (Sidhant Sharma's message of "Tue, 1 Mar 2016 23:18:49 +0530")
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/
next prev parent reply other threads:[~2016-03-01 17:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-01 15:36 [PATCH] builtin/receive-pack.c: use parse_options API Sidhant Sharma [:tk]
2016-03-01 17:22 ` Matthieu Moy
2016-03-01 17:48 ` Sidhant Sharma
2016-03-01 17:57 ` Matthieu Moy [this message]
2016-03-01 18:54 ` Eric Sunshine
2016-03-01 20:21 ` [PATCH v2] " Sidhant Sharma [:tk]
2016-03-01 20:31 ` Sidhant Sharma
2016-03-01 20:39 ` Matthieu Moy
2016-03-01 22:05 ` Junio C Hamano
2016-03-02 5:18 ` Sidhant Sharma
2016-03-02 8:51 ` Junio C Hamano
2016-03-02 8:23 ` Matthieu Moy
2016-03-02 9:53 ` Duy Nguyen
2016-03-02 13:53 ` Sidhant Sharma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=vpqio1613p1.fsf@anie.imag.fr \
--to=matthieu.moy@grenoble-inp$(echo .)fr \
--cc=git@vger$(echo .)kernel.org \
--cc=tigerkid001@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox