Re: [PATCH v2 2/5] commit-graph: remove redundant handling of -h

From: Taylor Blau <me@ttaylorr.com>
Date: Jul 19, 2021, 16:34 UTC
Message-ID: <YPWpiYInoJ/KyKhx@nand.local>
In-Reply-To: <25b24cc9-a7fc-1286-6bcf-1fdd21ee8c49@gmail.com>

On Sun, Jul 18, 2021 at 02:55:09PM +0200, Andrei Rybak wrote:

Show 5 quoted lines

> On 18/07/2021 09:58, Ævar Arnfjörð Bjarmason wrote:
> > I think this code added in 4ce58ee38d (commit-graph: create
> > git-commit-graph builtin, 2018-04-02) was always redundant,
> > parse_options() did this at the time, and the commit-graph code never
> > used PARSE_OPT_NO_INTERNAL_HELP.

Yep, that matches my findings.

Show 12 quoted lines

> > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> > index af88f805aa2..5fccce95724 100755
> > --- a/t/t5318-commit-graph.sh
> > +++ b/t/t5318-commit-graph.sh
> > @@ -5,6 +5,11 @@ test_description='commit graph'
> >   GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0
> > +test_expect_success 'usage' '
> > +	test_expect_code 129 git commit-graph -h 2>err &&
> > +	! grep error: err
>
> New test is partially redundant to the test in the loop at the bottom of
> 't0012-help.sh'.

Agreed. The change looks good to me, but we should drop this redundant test (although inspecting `git grep -w 'git .* -h' -- t` shows that there are a handful of other ones that could probably be dropped, too).

I don't really feel strongly enough to worry about cleaning up the existing ones, but we should strive to avoid adding new ones.

Thanks, Taylor