Re: What's cooking in git.git (Feb 2020, #04; Mon, 17)

[Elijah Newren <newren@gmail.com>]

On Fri, Feb 21, 2020 at 7:03 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Fri, Feb 21, 2020 at 8:26 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >
> > On 17/02/2020 22:08, Junio C Hamano wrote:
> > > Here are the topics that have been cooking.  Commits prefixed with
> > > '-' are only in 'pu' (proposed updates) while commits prefixed with
> > > '+' are in 'next'.  The ones marked with '.' do not appear in any of
> > > the integration branches, but I am still holding onto them.
> > >
> > > Accumulated fixes on the 'master' front have been flushed to 'maint'
> > > and a new maintenance release 2.25.1 has been tagged.
> > > [...]
> > > [Stalled]
> > >
> > > * pw/advise-rebase-skip (2019-12-06) 9 commits
> > >   - rebase -i: leave CHERRY_PICK_HEAD when there are conflicts
> > >   - rebase: fix advice when a fixup creates an empty commit
> > >   - commit: give correct advice for empty commit during a rebase
> > >   - commit: encapsulate determine_whence() for sequencer
> > >   - commit: use enum value for multiple cherry-picks
> > >   - sequencer: write CHERRY_PICK_HEAD for reword and edit
> > >   - cherry-pick: check commit error messages
> > >   - cherry-pick: add test for `--skip` advice in `git commit`
> > >   - t3404: use test_cmp_rev
> > >
> > >   The mechanism to prevent "git commit" from making an empty commit
> > >   or amending during an interrupted cherry-pick was broken during the
> > >   rewrite of "git rebase" in C, which has been corrected.
> > >
> > >   What's the status of this one?
> > >   The tip two are still RFC.
> >
> > The tip "rebase -i: leave CHERRY_PICK_HEAD when there are conflicts"
> > needs reworking and can be dropped (cf
> > <7e1b92f5-48df-e202-ebcc-5b15987a7d63@gmail.com>). The other RFC patch
> > "rebase: fix advice when a fixup creates an empty commit" [1] could do
> > with someone commenting on it (I've Cc'd dscho and Elijah). I think the
>
> I'll try to take a look early to middle of next week.
>
> > messages in it could be improved, but if the idea of different messages
> > for fixups that make a commit empty is not popular then the rest of the
> > series can be simplified by dropping some earlier patches including
> > patch 6 which you had some doubts about. (I tired to address those
> > <141f95b0-cae0-06f6-2c29-618dc22ae000@gmail.com> but I don't know if I
> > convinced you)

Sorry for the delay.  I can see how different messages might be useful
in the case where a fixup creates an empty commit, but I think it
would need more care since users won't know whether instructions are
referring to the combined changes of the original commit plus the
fixup, or if the instructions about skipping/dropping/whatever are
just referring to the changes from the fixup.  It wasn't clear to me.

Added a few other comments on that patch too.  Most of the patches
earlier in the series looked good to me; I was slightly
concerned/confused initially by patch 6 and part of patch 8 for
similar reasons on the determine_whence stuff.  This was only a minor
concern and I wouldn't hold up the patches over this, but I did I
throw an idea out there as an alternative -- though it might possibly
be worse.  (Nothing like throwing a bad idea out there to get creative
juices flowing, right?)  I skipped patch 9 since you already mentioned
it needs fixes.