From: Eric Sunshine <sunshine@sunshineco.com>
Date: Wed, 06 Apr 2016 06:21:38 GMT
Subject: Re: [PATCH v5 2/6] t7030-verify-tag: Adds validation for multiple tags
Message-ID: <CAPig+cSOLnCoX77O7khEX1cTh0Hu29d6MRdcpfN8ytTkUSVU=A@mail.gmail.com>
In-Reply-To: <1459872449-7537-3-git-send-email-santiago@nyu.edu>

On Tue, Apr 5, 2016 at 12:07 PM,  <santiago@nyu.edu> wrote:
> t7030-verify-tag: Adds validation for multiple tags

s/t7030-verify-tag/t7030/
s/Adds/add

> The verify-tag command supports multiple tag names as an argument.
> However, existing tests only test for invocation with a single tag, so
> we add a test invoking with multiple tags.
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Santiago Torres <santiago@nyu.edu>
> ---
> diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
> @@ -112,4 +112,16 @@ test_expect_success GPG 'verify signatures with --raw' '
> +test_expect_success GPG 'verify multiple tags' '
> +       tags="fourth-signed sixth-signed seventh-signed" &&
> +       for i in $tags; do

Style: 'do' on its own line; drop semicolon

    for i in ...
    do
        ...
    done

> +               git verify-tag -v --raw $i || return 1
> +       done >expect.stdout 2>expect.stderr.1 &&
> +       grep "^.GNUPG" <expect.stderr.1 >expect.stderr &&

Hmm, is there a reason you didn't stick with the more strict regex
Peff suggested?

    ^.GNUPG:.

(Genuine question: I'm not saying your choice is wrong, I'm just
interested in the reasoning behind the decision.)

> +       git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
> +       grep "^.GNUPG" <actual.stderr.1 >actual.stderr &&
> +       test_cmp expect.stdout actual.stdout &&
> +       test_cmp expect.stderr actual.stderr
> +'
> +
>  test_done