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

[Jacob Keller <jacob.keller@gmail.com>]

On Thu, Jun 16, 2022 at 2:31 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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.
>

Yea, I can re-order this. I generally opted to put it last since it
added code which would get modified. But yes, I agree it makes more
sense to be first.

> 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.

Yep, I'll re-order it.