From: Jeff King <peff@peff.net>
Date: Sat, 02 Aug 2014 00:04:05 GMT
Subject: Re: [PATCH v1 07/19] rebase -i: log the replay of root commits
Message-ID: <20140802000405.GB20040@peff.net>
In-Reply-To: <7389745946359e0d9d8c6f1e8f2ebc78033132e6.1406589435.git.bafain@gmail.com>

On Tue, Jul 29, 2014 at 01:18:07AM +0200, Fabian Ruch wrote:

> The command line used to recreate root commits specifies the option
> `-q` which suppresses the commit summary message. However,
> git-rebase--interactive tends to tell the user about the commits it
> creates in the final history, if she wishes (cf. command line option
> `--verbose`). The code parts handling non-root commits and squash
> commits all output commit summary messages. Do not make the replay of
> root commits an exception. Remove the option to make the report of
> the rebased history complete.
> 
> It is OK that the commit summary is still suppressed when git-commit
> is used to initialize the authorship of the sentinel commit because
> this additional commit is an implementation detail hidden from the
> final history. The removed `-q` option was probably introduced as a
> copy-and-paste error stemming from that part of the root commit
> handling code.

I'm confused. This implies that we should be seeing summaries for other
commits, but not root commits, and this patch is bring them into
harmony.  But if I have a repo like this:

  git init -q repo &&
  cd repo &&
  for i in one two; do
    echo $i >file &&
    git add file &&
    git commit -q -m $i
  done

then using stock git gives me this:

  $ GIT_EDITOR=true git rebase -i --root 2>&1 | perl -pe 's/\r/\\r\n/g'
  Rebasing (1/2)\r
  Rebasing (2/2)\r
  Successfully rebased and updated refs/heads/master.

but with your patch, I get:

  $ GIT_EDITOR=true git.compile rebase -i --root 2>&1 | perl -pe 's/\r/\\r\n/g'
  Rebasing (1/2)\r
  [detached HEAD 60834b3] one
   Date: Fri Aug 1 20:00:05 2014 -0400
   1 file changed, 1 insertion(+)
   create mode 100644 file
  Rebasing (2/2)\r
  Successfully rebased and updated refs/heads/master.

Am I misunderstanding the purpose of the patch?

-Peff