Re: [PATCH v3] submodule: port subcommand 'set-branch' from shell to C

From: Junio C Hamano <gitster@pobox.com>
Date: May 21, 2020, 18:44 UTC
Message-ID: <xmqqk115ruux.fsf@gitster.c.googlers.com>
In-Reply-To: <20200521163819.12544-1-shouryashukla.oo@gmail.com>

Shourya Shukla <shouryashukla.oo@gmail.com> writes:

Show 12 quoted lines

> Convert submodule subcommand 'set-branch' to a builtin and call it via
> 'git-submodule.sh'.
>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> Helped-by: Denton Liu <liu.denton@gmail.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---
> Thank you for the review Eric. I have changed the commit message,
> and the error prompts. Also, I have added a brief comment about
> the `quiet` option.

Sorry, I may have missed the previous rounds of discussion, but the comment adds more puzzles than it helps readers. "is currently not used" can be seen from the code, but it is totally unclear why it is not used. Is that a design decision to always keep quiet or always talkative (if so, "suppress output..." is not a good description)? Is that that this is a WIP patch that the behaviour the option aims to achieve hasn't been implemented? Is it that no existing callers pass "-q" to the scripted version, so there is no need to support it (if so, why do we even accept it in the first place)? Is it that all existing callers pass "-q" so we need to accept it, but there is nothing we need to make verbose so the variable is not passed around in the codepath?