git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Mike Crowe <mac@mcrowe.com>
Date
Dec 3, 2015, 13:10 UTC
Message-ID
<20151203131006.GA5119@mcrowe.com>
In-Reply-To
<xmqqsi3k4ety.fsf@gitster.mtv.corp.google.com>
On Wednesday 02 December 2015 at 15:21:13 -0800, Junio C Hamano wrote:
Show 27 quoted lines
> 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.

Show 5 quoted lines
> 
>     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.
Previous: Junio C HamanoNext: Mike Crowe
Message 6 of 16 in “push: add recurseSubmodules config option”
  1. push: add recurseSubmodules config optionMike Crowe, Dec 1, 2015
  2. Jeff KingDec 2, 2015
  3. Mike CroweDec 2, 2015
  4. push: Improve --recurse-submodules supportMike Crowe, Dec 2, 2015
  5. Junio C HamanoDec 2, 2015
  6. Mike CroweDec 3, 2015
  7. 1/2 push: Fully test --recurse-submodules on command line overrides configMike Crowe, Dec 3, 2015
  8. 2/2 push: Use "last one wins" convention for --recurse-submodulesMike Crowe, Dec 3, 2015
  9. Junio C HamanoDec 4, 2015
  10. Stefan BellerDec 10, 2015
  11. Junio C HamanoDec 10, 2015
  12. Stefan BellerDec 10, 2015
  13. Stefan BellerDec 16, 2015
  14. Junio C HamanoDec 16, 2015
  15. Stefan BellerDec 16, 2015
  16. Junio C HamanoDec 17, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.