From: Linus Arver <linusa@google.com>
Date: Fri, 22 Sep 2023 18:26:29 GMT
Subject: Re: [PATCH v3 13/13] trailer doc: <token> is a <key> or <keyAlias>, not both
Message-ID: <owly5y42t67e.fsf@fine.c.googlers.com>
In-Reply-To: <xmqq8r90dh2v.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>> @@ -248,34 +258,40 @@ With `add`, a new trailer will be added.
>>>  +
>>>  With `doNothing`, nothing will be done.
>>>  
>>> -trailer.<token>.key::
>>> -	This `key` will be used instead of <token> in the trailer. At
>>> -	the end of this key, a separator can appear and then some
>>> -	space characters. By default the only valid separator is ':',
>>> -	but this can be changed using the `trailer.separators` config
>>> -	variable.
>>> +trailer.<keyAlias>.key::
>>> +	Defines a <keyAlias> for the <key>. The <keyAlias> must be a
>>> +	prefix (case does not matter) of the <key>. For example, in `git
>>> +	config trailer.ack.key "Acked-by"` the "Acked-by" is the <key> and
>>> +	the "ack" is the <keyAlias>. This configuration allows the shorter
>>> +	`--trailer "ack:..."` invocation on the command line using the "ack"
>>> +	<keyAlias> instead of the longer `--trailer "Acked-by:..."`.
>>> ++
>>> +At the end of the <key>, a separator can appear and then some
>>> +space characters. By default the only valid separator is ':',
>>> +but this can be changed using the `trailer.separators` config
>>> +variable.
>>
>> I think all the other patches will be a great help to the user, but I'm
>> on the fence about this one. Someone who knows these trailer components
>> by their old names might be confused upon seeing tne new ones, so I'm
>> inclined to minimize such changes. I do think that the new names make
>> more sense, though.
>
> As long as the new names describe the world order better than the
> old description, I do not mind rephrasing the documentation, and you
> seem to find the more descriptive <keyAlias> easier to understand
> compared to the non-descriptive <token>.  Adding a concrete example
> (ack vs acked-by) is also a good change.

SGTM (i.e., I am OK to keep this patch as is). So I will not re-roll.

In the future I will be cleaning up more of the trailer system to make
use of the disinction between the key vs keyAlias where appropriate.

Thanks!