Re: [PATCH v2 0/5] support negative refspecs in git remote show

[Junio C Hamano <gitster@pobox.com>]

Jacob Keller <jacob.keller@gmail.com> writes:

> This series adds support for negative refspecs to git remote show, fixing an
> issue reported by Pavel Rappo.
>
> In addition, it includes some cleanup of the t5505-remote.sh test script,
> focusing on removing subshells and using test_config more.
>
> To support this, test_config and test_unconfig are extended to take and
> handle more options. The test_config_global is removed in favor of just
> using test_config --global.
>
> In addition, test_config now passes the value and --fixed-value into
> test_unconfig so that only the specific value is removed (rather than all
> keys of the name).
>
> The original v1 can be found here:
> https://lore.kernel.org/git/20220614003251.16765-1-jacob.e.keller@intel.com/
>
> If the config changes are too controversial, I'm happy to split them out
> into a separate series for further discussion, or drop them if they aren't
> desirable.

I did not see anything in 5/5 that substantially depends on all the
code churn done in 1/5-4/5.  Am I mistaken?

It would have been much nicer to organize the patch series so that
the first one is the [v2 5/5].  It may not be able to use the
improved test_config, but writing test_when_finished instead would
not be the end of the world.  The three-line test body will still be
three lines.  Then test_when_finished will have to be updated in
follow-up patches that corresponds to [v2 1/5]-[v2 4/5], but that is
the cost of "clean up".  The main "fix" patch shouldn't be the one
that is paying the price for it.

The clean-up offered by [v2 1/5] is a worthwhile thing to do.  It's
just that I do not think it is wise to make the fix in [v2 5/5] wait
for the 1.4k lines patch to be adequately reviewed.

Retiring "test_config_global" in [v2 2/5] is probably a good change,
especially when we are to add more featurs to test_config.  Again,
[v2 5/5] shouldn't have to be made waiting on an extra 800-line patch
to be reviewed.