On Fri, Dec 4, 2020 at 5:07 PM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras
> <felipe.contreras@xxxxxxxxx> wrote:
> >
> > This essentially reverts commit f15e7cf5cc.
> >
> > Once commit d9f15d37f1 introduced the autostash option for the merge
> > mode, it's not necessary to skip the fast-forward run_merge() when
> > autostash is set.
>
> It helps reviewers and future code readers if you provide a little
> context when referring to commits, making use of git log's
> --pretty=reference option to get the output.
Yes, I actually have this alias:
short = show --quiet --format='%C(auto)%h (%s)%C(reset)'
Which shows almost the same thing. I've updated it to
--format=reference. Thanks for the suggestion.
And I usually add those descriptions.
> So, for example, here
> your commit would read:
>
> """
> This essentially reverts commit f15e7cf5cc (pull: ff --rebase
> --autostash works in dirty repo, 2017-06-01).
>
> Once commit d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07)
> introduced the autostash option for the merge
> mode, it's not necessary to skip the fast-forward run_merge() when
> autostash is set.
> """
>
> I still found it slightly hard to follow the explanation even with the
> added summaries, though.
And that's the reason I didn't add them. You need to look at both the
commits to understand why one cancels the other. In my opinion in this
particular case the description only makes the text harder to read.
Probably some better $subjects like f15e7cf5cc (pull: skip ff merge
shortcut on --rebase --autostash) would have helped.
> An extra sentence at the end of the second
> paragraph to make it clear what is being changed ("So, change the code
> to fast-forward even when autostash is set.") seems to help.
OK. That's implied by "it's not necessary to skip the fast-forward"
but it's better to be explicit.
How about this:
Currently "git pull --rebase" takes a shortcut in the case a
fast-forward merge is possible; run_merge() is called with --ff-only.
However, "git merge" didn't have an --autostash option, so, when "git
pull --rebase --autostash" was called *and* the fast-forward merge
shortcut was taken, then the pull failed.
This was fixed in commit f15e7cf5cc (pull: ff --rebase --autostash
works in dirty repo, 2017-06-01) by simply skipping the fast-forward
merge shortcut.
Later on "git merge" learned the --autostash option [a03b55530a
(merge: teach --autostash option, 2020-04-07)], and so did "git pull"
[d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07)].
Therefore it's not necessary to skip the fast-forward merge shortcut
anymore when called with --rebase --autostash.
Let's always take the fast-forward merge shortcut by essentially
reverting f15e7cf5cc.
Cheers.
--
Felipe Contreras
