Re: [PATCH] fixup! log: add exhaustive tests for pattern style options & config

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2017, 00:46 UTC
Message-ID: <xmqq37c56334.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.21.1.1705151418000.146734@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 9 quoted lines

> On Mon, 15 May 2017, Junio C Hamano wrote:
>
>> My knee-jerk reaction matched Dscho's, but grep is about contents,
>> and we should be able to test this if we used a sensible tagnames or
>> didn't use any.  Glad to see somebody can step back and think ;-)
>
> Maybe somebody should step back even further and think even more, as we
> could adjust test_commit to mangle the argument into a tag name that is
> legal even with a refs backend relying on NTFS.

Perhaps, but I am not sure if that is needed.

The point of the helper is to serve as a simple "we are building a toy sample history by only adding a one-liner new file" convenience helper, and I think it is sensible to keep its definition simple. The callers (like the ones being added in the rerolled patch under discussion) with special needs can supply tagname when the default one is not suitable.

In hindsight, perhaps it would have been better if the default for the helper were _not_ to create any tag (and callers who care about tags can optionally tell it to add tag, or tag the resulting commit themselves), but that is lamenting water under the bridge.