Re: [PATCH v2 0/4] Let the builtin rebase call the git am command directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Fri, 18 Jan 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
> 
> > Especially on Windows, where Unix shell scripting is a foreign endeavor, and
> > an expensive one at that, we really want to avoid running through the Bash.
> >
> > This not only makes everything faster, but also more robust, as the Bash we
> > use on Windows relies on a derivative of the Cygwin runtime, which in turn
> > has to jump through a couple of hoops that are sometimes a little too tricky
> > to make things work. Read: the less we rely on Unix shell scripting, the
> > more likely Windows users will be able to enjoy our software.
> >
> > Changes since v1:
> >
> >  * Rebased on top of master to avoid merge conflicts.
> 
> I do not appreciate this very much.  
> 
> We already have a good resolution prepared when merging this to
> either 'next' or 'master', which also resolves conflict with the
> other topic that requires this topic to add "--topo-order" in its
> call.  Rebasing series means invalidating the previous work recorded
> in rerere.

You misunderstand. The PR had merge conflicts with `master`. As such, the
Azure Pipeline that tests it on Windows, macOS and Linux could not give me
enough confidence. So I *had* to rebase.

Besides, I think it is a terrible idea to leave the resolution of merge
conflicts to you. You are not in the best position to judge how to resolve
them, and as a consequence you spend more time and effort on this than
necessary. Of course, it's your call if you want to do it, as a
contributor I'd rather be certain that *I* resolved the conflicts.

Ciao,
Dscho

> 	side note. The rerere database entry can be recovered from
> 	master..pu with contrib/rerere-train.sh).
> 
> >  * Adjusted the commit message talking about double entries, to clarify that
> >    it talks about HEAD's reflog.
> 
> Good.
> 
> >  * Replaced a misleading action parameter "checkout" for the reset_head() 
> >    function by the empty string: we do not check out here, we just update
> >    the refs, and certainly do not want any checkout functionality (such as
> >    hooks) to be involved.
> 
> OK.
> 
> >  * Reused a just-prepared refs_only variable instead of repeating the value
> >    assigned to it.
> 
> OK.
> 
> >  * Fixed a stale comment about the shell variable "$upstream" (which should
> >    have been negated to begin with).
> >  * Fixed error messages when files could not be opened.
> 
> Good.
> 
> Will take a look.  Thanks.
> 
> 



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux