Re: [PATCH v3 0/3] Introduce log.showSignature config variable

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 24, 2016, 12:25 UTC
Message-ID: <CAPig+cSHRWHUJ+P9nxQXz5YGigD7XOT24oUJs=JSR5L+xDoc0A@mail.gmail.com>
In-Reply-To: <CA+DCAeSvb14W5CoEa72DQtM+FHQ+jppSVvoTmQ7rX0EHQAMy6g@mail.gmail.com>

On Fri, Jun 24, 2016 at 5:21 AM, Mehul Jain <mehul.jain2029@gmail.com> wrote:

Show 18 quoted lines

> On Thu, Jun 23, 2016 at 12:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Mehul Jain <mehul.jain2029@gmail.com> writes:
>>> In patch 2/3 and 3/3, there are many tests which requires a branch
>>> similar to that of "signed" branch, i.e. a branch with a commit having
>>> GPG signature. So previously in v2, I created two new branches,
>>> "test_sign" and "no_sign", which are identical to that of "signed"
>>> branch. And with these branches, I wrote the tests in patch 2/3
>>> and 3/3.
>>>
>>> As suggested by Eric [1], rather than creating new branches, I
>>> can take advantage of "signed" branch which already exists.
>>
>> Yeah, I understand that part.  But you do not _need_ to do the split
>> you do in 1/3 in order to reuse "signed".
>
> If it's fine, then I think it would be OK to drop this 1/3. Without splitting
> the 'log --graph --show-signature' in two test will also serve the
> purpose for the new test to use the signed branch.

My understanding of Junio's response is that the missing PGP prerequisite along with a weak commit message make for poor justification of patch 1/3, however, if you add the prerequisite and use the commit message he proposed (reproduced below) then it becomes sensible to retain 1/3.

    --->8---
    In 2/3 and 3/3, we will use the same 'signed' branch that the
    first test for 'log --graph --show-signature' uses.  This branch
    is currently created in that 'log --graph --show-signature' test
    itself.

    Split the set-up part into a test of its own, and make the
    existing first test into a separate one that only inspects the
    history on the 'signed' branch.  That way, it would become
    clearer that later tests added by 2/3 and 3/3 reuse the 'signed'
    branch in the same way this 'log --graph --show-signature' uses
    that same branch.
    --->8---