Re: [PATCH v4 2/5] rev-list: allow bisect and first-parent flags

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 5, 2020, 00:38 UTC
Message-ID: <CAPig+cQumRSCQA3Et5=h7SD7zqMm_Z6LJVUTonkewR=hNR55Ug@mail.gmail.com>
In-Reply-To: <20200804220113.5909-3-alipman88@gmail.com>

On Tue, Aug 4, 2020 at 6:01 PM Aaron Lipman <alipman88@gmail.com> wrote:

Show 9 quoted lines

> Add first_parent_only parameter to find_bisection(), removing the
> barrier that prevented combining the --bisect and --first-parent flags
> when using git rev-list
>
> Signed-off-by: Aaron Lipman <alipman88@gmail.com>
> ---
> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh
> @@ -263,4 +263,49 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '
> +test_output_expect_success "--bisect --first-parent" 'git rev-list --bisect --first-parent E ^F' <<EOF

Custom is to use single quotes around the test title, not double quotes (unless you need to interpolate a variable). Same comment applies to other tests introduced by this patch.

Show 13 quoted lines

> +test_expect_success "--bisect-all --first-parent" '
> +cat > expect.unsorted <<EOF &&
> +$(git rev-parse E) (tag: E, dist=0)
> +[...]
> +$(git rev-parse e8) (tag: e8, dist=1)
> +EOF
> +
> +# expect results to be ordered by distance (descending),
> +# commit hash (ascending)
> +sort -k4,4r -k1,1 expect.unsorted > expect &&
> +git rev-list --bisect-all --first-parent E ^F > actual &&
> +test_cmp actual expect
> +'

Indent this test body with TAB rather than having it sit at the left margin. You can use <<-EOF for the here-doc to allow its content to be indented, as well.

Swap the arguments to test_cmp.

Also, for style consistency, no whitespace following redirect:

    cat >expect.unsorted <<-EOF
    ...
    EOF
    ...
    sort ... >expect &&
    git rev-list ... >actual &&
    test_cmp expect actual