Re: [PATCH 1/3] pull: cleanup autostash check

From: Felipe Contreras <felipe.contreras@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Philip Oakley <philipoakley@iee.email>,
	Alex Henrie <alexhenrie24@gmail.com>, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/3] pull: cleanup autostash check
Date: Tue, 15 Jun 2021 05:59:02 -0500	[thread overview]
Message-ID: <60c887f678c88_e63320846@natae.notmuch> (raw)
In-Reply-To: <CABPp-BGstXDbzxpySw7q_jn22HD05MsrZeHNv+kXFHOFS2_WCQ@mail.gmail.com>

Elijah Newren wrote:
> On Sat, Jun 12, 2021 at 9:59 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > 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.
> >
> > Reviewed-by: Elijah Newren <newren@gmail.com>
> 
> I think you are basing the Reviewed-by on
> https://lore.kernel.org/git/CABPp-BEsQWsHMAmwc3gmJnXcS+aR-FtoMJxBRQ=BpARP49-L-Q@mail.gmail.com/;
> is that correct?

No, more like:

[1] https://lore.kernel.org/git/20201205195313.1557473-5-felipe.contreras@gmail.com/

> Messages from folks that they seem to like the patch
> or believe it looks good should be translated into an Acked-by rather
> than a Reviewed-by; from Documentation/SubmittingPatches:

To me an acknowledgment means something entirely different, and must be
expressly given.

> * `Reviewed-by:`, unlike the other tags, can only be offered by the
>   reviewer and means that she is completely satisfied that the patch
>   is ready for application.  It is usually offered only after a
>   detailed review.

Yeah, I read that after I sent v3. In this series I simply cherry-picked
it from a previous series.

I guess I'll just avoid both.

-- 
Felipe Contreras