Re: [PATCH 06/34] sequencer (rebase -i): write the 'done' file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dennis,

On Wed, 31 Aug 2016, Dennis Kaarsemaker wrote:

> On wo, 2016-08-31 at 10:54 +0200, Johannes Schindelin wrote:
> > In the interactive rebase, commands that were successfully processed are
> > not simply discarded, but appended to the 'done' file instead. This is
> > used e.g. to display the current state to the user in the output of
> > `git status` or the progress.
> 
> Wouldn't it make more sense to have this patch before the ones that
> implement the actual rebase commands?

I waffled about the order so many times that I don't know anymore. The
thing is, while the sequencer is taught incrementally to understand all of
the rebase -i functionality, rebase -i itself is not touched, on purpose.

In the case of the "done" file, my thoughts were: the commands do not need
this file *at all*. In fact, if we did not write the "done" file at all,
the only two types of test failures in the test suite would be 1) git
status' output and 2) the prompt testing for the progress.

So you see, functionally, the "done" file is only relevant to the progress
part of the patch series.

As such, I'd rather keep this patch in the current place, just before
introducing the progress.

> Hmm, and after reading more of this series, I think the same applies to
> some other patches too, e.g. 08/34 and 14/34, so I'm probably missing
> something. So before I make a fool of myself and suggest that the
> implementation of the actual commands should come at the end, maybe you
> could tell me what I'm missing :) 

No, no, don't hesitate to suggest reorderings. I am really thankful for
the discussion we are having, so that the outcome is better than what I
have right now. If the outcome would be the very same patches, but with
more confidence, it would still be better than what I have right now ;-)

Ciao,
Dscho

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]