Re: [PATCH] Don't pass -v to submodule command

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 2, 2022, 00:24 UTC
Message-ID: <xmqqiliur6t9.fsf@gitster.g>
In-Reply-To: <221130.868rjsi6bn.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 12 quoted lines

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

Show 22 quoted lines

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