Hi Junio
On 09/12/2021 21:04, Junio C Hamano wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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()?
So the theme started out as "convert 'rebase -i' to use reset_head() and
stop forking 'git checkout'" which I thought would be one to two
patches. Then I started looking at the code and realized that
reset_head() needed some work before we could use it for 'rebase -i' and
that work ended up dominating the series. I've updated the cover letter
as you suggested.
Best Wishes
Phillip
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.
