Re: [PATCH v4 03/13] commit-graph: create git-commit-graph builtin

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 23, 2018, 16:07 UTC
Message-ID: <c9d38f72-536b-887b-eaa7-3395746ef8e3@gmail.com>
In-Reply-To: <xmqqd10yyxsu.fsf@gitster-ct.c.googlers.com>

On 2/21/2018 1:58 PM, Junio C Hamano wrote:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Derrick Stolee <stolee@gmail.com> writes:
>>
>>> +int cmd_commit_graph(int argc, const char **argv, const char *prefix)
>>> +{
>>> +	static struct option builtin_commit_graph_options[] = {
>>> +		{ OPTION_STRING, 'p', "object-dir", &opts.obj_dir,
>>> +			N_("dir"),
>>> +			N_("The object directory to store the graph") },
>> I have a suspicion that this was modeled after some other built-in
>> that has a similar issue (perhaps written long time ago), but isn't
>> OPT_STRING() sufficient to define this element these days?
>>
>> Or am I missing something?

You are not. There are several places in this history of this patch where I was using old patterns because I was using old code as my model (places like 'index-pack').

>> Why squat on short-and-sweet "-p"?  For that matter, since this is
>> not expected to be end-user facing command anyway, I suspect that we
>> do not want to allocate a single letter option from day one, which
>> paints ourselves into a corner from where we cannot escape.

I'll drop all single-letter shortcuts.

> I suspect that exactly the same comment applies to patches in this
> series that add other subcommands (I just saw one in the patch for
> adding 'write').
>

Thanks, -Stolee