From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Sven Strickroth <email@cs-ware.de>, git <git@vger.kernel.org>,
"Robin H. Johnson" <robbat2@gentoo.org>
Subject: Re: [PATCH] Don't pass -v to submodule command
Date: Fri, 02 Dec 2022 09:24:34 +0900 [thread overview]
Message-ID: <xmqqiliur6t9.fsf@gitster.g> (raw)
In-Reply-To: <221130.868rjsi6bn.gmgdl@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Wed, 30 Nov 2022 20:17:23 +0100")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On Wed, Nov 30 2022, Sven Strickroth wrote:
>
>> "git pull -v --recurse-submodules" propagates the "-v" to the submdoule
>> command which does not support "-v".
>>
>> Commit a56771a668dd4963675914bc5da0e1e015952dae introduced this
>> regression.
>
> We refer to commits in commit messages like this: a56771a668d
> (builtin/pull: respect verbosity settings in submodules, 2018-01-25);
>
> Which also shows that this regression is quite old.
Good point.
While we are commenting on the proposed log message, this subject
> Subject: [PATCH] Don't pass -v to submodule command
is not sufficient to identify the change and remind readers what it
is about when it is shown among "git shortlog --no-merges". We give
"<area>:" prefix before the title to help that, e.g.
Subject: [PATCH] pull: don't pass -v to "git submodule update"
or something like that.
>> Signed-off-by: Sven Strickroth <email@cs-ware.de>
>> ---
>> builtin/pull.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/pull.c b/builtin/pull.c
>> index 1ab4de0005..b67320fa5f 100644
>> --- a/builtin/pull.c
>> +++ b/builtin/pull.c
>> @@ -256,7 +256,7 @@ static struct option pull_options[] = {
>> /**
>> * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
>> */
>> -static void argv_push_verbosity(struct strvec *arr)
>> +static void argv_push_verbosity(struct strvec *arr, int include_v)
>> {
>> int verbosity;
>>
>
> It looks like you're getting somewhere with this, but you never use this
> "include_v", so the bug is still there. We just have the scaffolding
> now.
What is the plan to cope with the evolution of "git submodule
update" command, though? Will "-v" forever be the single option we
may get at "git pull" level that will never be supported by "git
submodule update"? I am guessing that the reason we want to call
this flag "include_v" is because it is the author's intention that
"git submodule update" will not change in this regard, and am
wondering if that is a healthy assumption.
> Did you forget to add that part to this commit?
>
> In any case, that serves as a comment on the other thing this patch
> really needs: tests, please add some.
Good advice.
> I think the right longer term fix here is to simply make "git submodule"
> support "-v" and "--verbose".
Yup.
next prev parent reply other threads:[~2022-12-02 0:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-24 12:47 git pull --verbose with submodules ends in error message Fink, Mike
2022-11-25 15:56 ` Sven Strickroth
2022-11-30 18:30 ` [PATCH] Don't pass -v to submodule command Sven Strickroth
2022-11-30 19:17 ` Ævar Arnfjörð Bjarmason
2022-12-01 8:32 ` Sven Strickroth
2022-12-01 8:34 ` [PATCH v2] " Sven Strickroth
2022-12-02 0:24 ` Junio C Hamano [this message]
2022-12-10 13:06 ` [PATCH] submodule: Accept -v for update command Sven Strickroth
2022-12-18 1:25 ` Junio C Hamano
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqiliur6t9.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=email@cs-ware.de \
--cc=git@vger.kernel.org \
--cc=robbat2@gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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
https://80x24.org/mirrors/git.git
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).