mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Glen Choo <>
To: Jonathan Tan <>
Cc: Jonathan Tan <>,,
Subject: Re: [PATCH v2] Doc: document push.recurseSubmodules=only
Date: Wed, 16 Nov 2022 13:21:03 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Jonathan Tan <> writes:

>> When we pass this magic, undocumented value, "git push" will warn about
>> about "only" and override it with "on-demand". We always pass it when we
>> recurse into submodules, and we assume that no user will pass it, thus
>> we get the warning iff we are recursing into submodules.
>> In that case, it sounds like "--recurse-submodules=only-is-on-demand" is
>> a synonym for "this is a submodule that is being recursed into". In that
>> case, wouldn't it be more self-documenting to have a hidden CLI flag
>> that expresses exactly that ? e.g. we could add a PARSE_OPT_HIDDEN flag
>> called "--is-recursing" and check that boolean value. This seems clearer
>> to me at least.
> Hmm...--recurse-submodules=only-is-on-demand is hidden too, right?

I suppose so, but as a matter of personal taste, when encountering a
hidden CLI option, I'd prefer to see the option's documentation when I

  OPT_HIDDEN_BOOL(0, "is-recursing", &is_recursing, "internal only,
    override recurseSubmodules = only")


  if (!strcmp(arg, "only-is-on-demand")) {
    /* etc */

>                                                                    One
> advantage of doing this instead of a separate arg is that neither the
> caller nor "git push" needs to think about what happens if both --recurse-
> submodules=something and --is-recursing are both specified.

That's true, and I suppose it prevents the temptation to reuse
--is-recursing for things it wasn't meant for. But in that case,
couldn't we say the same thing if we renamed "--is-recursing" to

At any rate, do treat this as just a suggestion. I won't block this if
both you and Taylor find this clear enough :)

  reply	other threads:[~2022-11-16 21:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08  0:25 [PATCH] submodule: explicitly specify on-demand upon push Jonathan Tan
2022-11-08  0:31 ` Taylor Blau
2022-11-08 21:43   ` Jonathan Tan
2022-11-09 18:31     ` Glen Choo
2022-11-14 21:37 ` [PATCH v2] Doc: document push.recurseSubmodules=only Jonathan Tan
2022-11-14 21:57   ` Taylor Blau
2022-11-15  0:59   ` Glen Choo
2022-11-15 18:47     ` Jonathan Tan
2022-11-16 21:21       ` Glen Choo [this message]
2022-11-16 22:35         ` Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).