Re: [PATCH] push: Improve --recurse-submodules support

[Mike Crowe <mac@mcrowe.com>]

On Wednesday 02 December 2015 at 15:21:13 -0800, Junio C Hamano wrote:
> Mike Crowe <mac@mcrowe.com> writes:
> 
> > b33a15b08131514b593015cb3e719faf9db20208 added support for the
> > push.recurseSubmodules config option. After it was merged Junio C Hamano
> > suggested some improvements:
> >
> >  - Declare recurse_submodules on a separate line.
> >
> >  - Accept multiple --recurse-submodules options on command line with the
> >    last one winning. (This simplified the implementation too.)
> >
> > Also slightly improve one of the tests added in
> > b33a15b08131514b593015cb3e719faf9db20208.
> 
> The above is overly verbose about how the commit materialized,
> compared to the description of the merit of this update.
> 
>     push: fix --recurse-submodules breakage
>
>     When b33a15b0 (push: add recurseSubmodules config option,
>     2015-11-17) added push.recurseSubmodules configuration option,
>     it also changed the command line parsing to allow
>     --no-recurse-submodules to override configured default.
>
>     However, the parsing of configuration variables and command line
>     options did not follow the usual "last one wins" convention.
>     Fix this.

That's not quite true.

The check for conflicting options was added back in 2012 by eb21c732 when
--recurse-submodules=on-demand support was originally implemented. b33a15b0
treated that as correct and maintained the behaviour.

> 
>     Also fix the declaration of the new file-scope global variable
>     to put it on a separate line on its own.
> 
> or something?

Thanks for the better wording. Hopefully I've included enough of the right
bits in the updated patches that follow.

> Also describe what "slightly improve" really means.  What did the
> old one not test that should have been tested?

In attempting to describe this change I've found that both of the tests had
failings so I have improved them too.

Thanks.

Mike.