Re: [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 22, 2016, 16:19 UTC
Message-ID: <xmqqd1is9x0s.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAP8UFD0XvNx6Brio9muDhOj5+dr=HRbhK_Lmr+k4LUPKrAVC9Q@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 19 quoted lines

> On Fri, Oct 21, 2016 at 2:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> If I were guiding a topic that introduce this feature from scratch
>> today, I would probably suggest a pattern based approach, e.g.  a
>> built-in "[-A-Za-z0-9]+:" [*1*] may be the default prefix that is
>> used to recognize the beginning of a trailer, and a user or a
>> project that wants "Bug #538" would be allowed to add an additional
>> pattern, e.g. "Bug *#", that recognises a custom trailer line that
>> is used by the project.
>
> When we designed the separator mechanism, we had the following discussions:
>
> https://public-inbox.org/git/xmqqa9a1d6xn.fsf@gitster.dls.corp.google.com/
> https://public-inbox.org/git/xmqqmwcuzyqx.fsf@gitster.dls.corp.google.com/
>
> They made me think that you were against too much flexibility, so I
> removed functionality that allowed to put separators into the ".key"
> config options, and now you are saying that we botched the thing and
> that you would like more flexibility of this kind back.

Correct.  Pay attention to the fact that I said _we_ botched.

If an initial design made by a topic author is crappy, that may be author's botch. Once a topic goes through a review cycle by getting reviewed, rerolled, re-reviewed, ... to the point that those involved accept the result, and we later realize that it was not good, the botch no longer is author's alone. If it is shipped as part of a release, then it is not just the authors and the reviewers but everybody. We collectively stopped at a place that was not ideal and share the blame ;-).

> Anyway I think it is still possible to add back such kind of
> functionality in a backward compatible way for example by adding
> ".extendedKey" config options.

Yup, or with trailer.keyPattern that is multi-values, or with any number of alternatives.