Re: [PATCH RFC v2 07/19] rebase -i: The replay of root commits is not shown with --verbose

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2014, 22:29 UTC
Message-ID: <xmqq4myr8oxz.fsf@gitster.dls.corp.google.com>
In-Reply-To: <b1ecef042cd18a0251199429e4efb969b085d5fe.1404323078.git.bafain@gmail.com>

Fabian Ruch <bafain@gmail.com> writes:

Show 17 quoted lines

> The command line used to recreate root commits specifies the
> erroneous option `-q` which suppresses the commit summary message.
> However, git-rebase--interactive tends to tell the user about the
> commits it creates, if she wishes (cf. command line option
> `--verbose`). The code parts handling non-root commits or squash
> commits all output commit summary messages. Do not make the replay of
> root commits an exception. Remove the option.
>
> It is OK to suppress the commit summary when git-commit is used to
> initialize the authorship of the sentinel commit because the
> existence of this additional commit is a detail of
> git-rebase--interactive's implementation. The option `-q` was
> probably introduced as a copy-and-paste error stemming from that part
> of the root commit handling code.
>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
> ---

This one I can buy; there is no reason to drop "-q" from both (which would give us the same thing twice and the one before the "pick_one -n" runs is not the final one anyway) and the later one that records the updated tree would be the one to report what it did.

Show 16 quoted lines

>  git-rebase--interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0af96f2..ff04d5d 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -511,7 +511,7 @@ do_pick () {
>  			   --no-post-rewrite -n -q -C $1 &&
>  			pick_one -n $1 &&
>  			git commit --allow-empty \
> -				   --amend --no-post-rewrite -n -q -C $1 \
> +				   --amend --no-post-rewrite -n -C $1 \
>  				   ${gpg_sign_opt:+"$gpg_sign_opt"} ||
>  			die_with_patch $1 "Could not apply $1... $2"
>  	else