Hi Phillip,

On Thu, 3 Aug 2023, Phillip Wood wrote:

> On 27/03/2023 08:04, Johannes Schindelin wrote:
> >
> > On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote:
> >
> > > From: Phillip Wood <phillip.wood@dunelm.org.uk>
> > >
> > > As the sequencer executes todo commands it appends them to
> > > .git/rebase-merge/done. This file is used by "git status" to show the
> > > recently executed commands. Unfortunately when a command is rescheduled
> > > the command preceding it is erroneously appended to the "done" file.
> > > This means that when rebase stops after rescheduling "pick B" the "done"
> > > file contains
> > >
> > >  pick A
> > >  pick B
> > >  pick A
> > >
> > > instead of
> > >
> > >  pick A
> > >  pick B
> > >
> > > Fix this by not updating the "done" file when adding a rescheduled
> > > command back into the "todo" file. A couple of the existing tests are
> > > modified to improve their coverage as none of them trigger this bug or
> > > check the "done" file.
> >
> > I am purposefully not yet focusing on the patch, as I have a concern about
> > the reasoning above.
> >
> > When a command fails that needs to be rescheduled, I actually _like_ that
> > there is a record in `done` about said command. It is very much like a
> > `pick` that failed (but was not rescheduled) and was then `--skip`ed: it
> > still shows up on `done`.
>
> We still do that after this patch. What changes is that when "pick B" fails we
> don't add "pick A" to the "done" file when "pick B" is added back into
> "git-rebase-todo"
>
> > I do understand the concern that the rescheduled command now shows up in
> > both `done` and `git-rebase-todo` (which is very different from the failed
> > `pick` command that would show up _only_ in `git-rebase-todo`). So maybe
> > we can find some compromise, e.g. adding a commented-out line to `done` à
> > la:
> >
> >  # rescheduled: pick A
> >
> > What do you think?
>
> If a commit is rescheduled we still end up with multiple entries in the
> "done". In the example above if "pick B" fails the first time it is executed
> and then succeeds on the second attempt "done" will contain
>
> 	pick A
> 	pick B
> 	pick B
>
> It might be nice to mark it as rescheduled as you suggest but this series
> focuses on removing the incorrect entry from the "done" file, not
> de-duplicating the "done" entities when a command fails.

After having had plenty of time to let this issue simmer in the back of my
head, and after reviewing the latest iteration of the patch series, I am
no longer concerned, as it now sounds more logical to me, too, that
rescheduled commands don't show up in `done`.

Thank you,
Johannes