Subject: Re: [PATCH v2 00/14] rebase: reset_head() related fixes and improvements

From: Junio C Hamano <[email-masked]>
Date: Thu, 09 Dec 2021 13:04:17 -0800

"Phillip Wood via GitGitGadget" <[email-masked]> writes:

> Thanks for the comments on V1. I have tried to improve the commit messages
> to explain better the motivation and implications of the changes in this
> series and I have added some more tests. I have rebased onto v2.34.0 to
> avoid some merges conflicts.

Thanks.

It still is not clear in the cover letter what the overall theme of
the topic is, and the original cover letter was deliberately vague
by saying "Fix *some* issues".  Random assortment of changes to
various code paths, the only common trait among them being they are
somehow related to reset_head()?

> Cover letter for V1: Fix some issues with the implementation and use of
> reset_head(). The last patch was previously posted as [1], I have updated
> the commit message and rebased it onto the fixes in this series. There are a
> couple of small conflicts merging this into seen, I think they should be
> easy to resolve (in rebase.c take both sides in reset.c take the changed
> lines from each side). These patches are based on pw/rebase-of-a-tag-fix

I've read the patches through.  It does revolve around the use of
reset_head().  I would have appreciated if the cover letter said
something along this line:

    reset.c::reset_head() started its life at ac7f467f
    (builtin/rebase: support running "git rebase <upstream>",
    2018-08-07) as a way to detach the HEAD to replay the commits
    during "git rebase", but over time it learned to do many things,
    like switching the tip of the branch to another commit,
    recording the old value of HEAD in ORIG_HEAD while it does so,
    recording reflog entries for both HEAD and for the branch.

    The API into the function got clunky and it is harder than
    necessary for the callers to use the function correctly, which
    led to a handful of bugs that this series is going to fix.

       ... list of bugs here ...

    Later steps of this series revamps the API so that it is harder
    to use it incorrectly to prevent future bugs.

Anyway, I think the series is more or less in a very good shape,
even though a few comments I threw at this round may result in a
further improvement.

Thanks for working on this.