Message ID | pull.24.v2.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Let the builtin rebase call the git am command directly | expand |
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> 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. 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.
Junio C Hamano <gitster@pobox.com> writes: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > 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. > > side note. The rerere database entry can be recovered from > master..pu with contrib/rerere-train.sh). I prepared a set of patches rebased back on top of the previous base and applied them on top of the previous base (call the result X). Applying the posted patches directly on top of master, and merging X into master, produces the same tree. So I'll keep the X (i.e. these patches backward-rebased to the previous base) and replace js/rebase-am with it. That is safer when recreating 'pu' and merging it to 'next', which I hope we can do soonish. > Will take a look. Thanks. They were pleasant and smooth read. Crafted nicely. Thanks.
Hi Junio, On Fri, 18 Jan 2019, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > 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. > >