"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
> In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02), we
> introduced a helpful message that suggests to run git cherry-pick --skip
> (instead of the previous message that talked about git reset) when a
> cherry-pick failed due to an empty patch.
>
> However, the same message is displayed during a rebase, when the patch
> to-be-committed is empty. In this case, git reset would also have worked,
> but git cherry-pick --skip does not work. This is a regression introduced in
> this cycle.
>
> Let's be more careful here.
>
> Johannes Schindelin (3):
> cherry-pick: add test for `--skip` advice in `git commit`
> sequencer: export the function to get the path of `.git/rebase-merge/`
> commit: give correct advice for empty commit during a rebase
Overall they looked nicely done. The last one may have started its
life as "let's fix advice for empty", but no longer is.
The old code used the sequencer_in_use boolean to tell between two
states ("are we doing a single pick, or a range pick?"), but now we
have another boolean rebase_in_progress, and depending on the shape
of the if statements existing codepaths happen to have, these two
are combined in different ways to deal with new states. E.g. some
places say
if (rebase_in_progress && !sequencer_in_use)
we are doing a rebase;
else
we are doing a cherry-pick;
and some others say
if (sequencer_in_use)
we are doing a multi pick;
else if (rebase_in_progress)
we are doing a rebase;
else
we are doing a single pick;
I wonder if it makes the resulting logic simpler to reason about if
these combination of two variables are rewritten to use a single
variable that enumerates three (or is it four, counting "we are
doing neither a cherry-pick or a rebase"?) possible state.
Other than that, looked sensible. Will queue.
Thanks.
