Re: [PATCH] gitk: use --pretty=reference for copysummary

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 11, 2019, 22:05 UTC
Message-ID: <xmqq1rtao6w6.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20191211215826.GA31614@blackberry>

Paul Mackerras <paulus@ozlabs.org> writes:

Show 18 quoted lines

> On Wed, Dec 11, 2019 at 01:39:50PM -0800, Denton Liu wrote:
>> In an earlier commit[1], git learned the 'reference' pretty format.
>> Update copysummary to use this pretty format instead of manually
>> reimplementing it as a format string.
>> 
>> With this change, we lose the double-quotes surrounding the commit
>> subject but it seems the consensus is that the unquoted form is used
>> more often anyway[2] so this change should be acceptable.
>> 
>> Since gitk and git are usually packaged and distributed together, their
>> versions should be in sync so we should not have to worry a newer gitk
>> running on top of an older version of git that doesn't support the
>> 'reference' pretty format.
>
> In fact my policy is not to do this (introduce a change to gitk that
> means it requires the very latest git).  I would want the code either
> to test the git version (which the code already does in other places)
> or handle failure gracefully and fall back to the old command.

For a case like this one, the policy would mean that a single liner patch like this will never be accepted, right? After all, the code that would be used as a fallback for older Git is very simple so it is almost pointless to add a check for feature with conditional. We can just use the fallback code always, which is essentially to keep the current code.

It is a tangent, but arguably the current code is easier to extend. I can even see somebody arguing for adding a gitk.summaryformat configuration variable, whose value would default to "%h (%s, %ad)" when missing---that can be quite straightforward to do without Denton's patch.

So I dunno.