From: Taylor Blau <me@ttaylorr.com>
Date: Mon, 19 Jul 2021 16:53:15 GMT
Subject: Re: [PATCH v2 5/5] commit-graph: show usage on "commit-graph [write|verify] garbage"
Message-ID: <YPWt++xs6lrqiExV@nand.local>
In-Reply-To: <patch-5.5-57ffd5812d6-20210718T074936Z-avarab@gmail.com>

On Sun, Jul 18, 2021 at 09:58:09AM +0200, Ævar Arnfjörð Bjarmason wrote:
> Change the parse_options() invocation in the commit-graph code to make
> sense. We're calling it twice, once for common options parsing, and
> then for the sub-commands.
>
> But we never checked if we had something leftover in argc in "write"
> or "verify", as a result we'd silently accept garbage in these
> subcommands. Let's not do that.

All makes sense and looks good to me. One small note below:

>  test_expect_success 'usage' '
>  	test_expect_code 129 git commit-graph -h 2>err &&
> -	! grep error: err
> +	! grep error: err &&

Ah, now I see why you added this test back in the first patch. I still
think that we should get rid of the first two lines, but...

> +	test_expect_code 129 git commit-graph write blah &&
> +	test_expect_code 129 git commit-graph write verify

Keeping these makes sense.

Thanks,
Taylor