Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
Jeff Kingwrites: > On Wed, Sep 21, 2016 at 07:45:50PM +0200, Kevin Daudt wrote: > >> On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote: >> > Kevin Daudt writes: >> > >> > > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote: >> > >> >> > >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits >> > >> - mailinfo: unescape quoted-pair in header fields >> > >> - t5100-mailinfo: replace common path prefix with variable >> > > >> > > Is this good enough, or do you want me to look into the feedback from >> > > jeff? >> > >> > If you are talking about the simplified loop that deliberately sets >> > a rule that is looser than RFC, yes, I'd like to see you at least >> > consider the pros and cons of his approach, which looked nicer to my >> > brief reading of it. >> > >> > It is perfectly OK by me (it may not be so if you ask Peff) if you >> > decide that your version is better after doing so, though. >> >> Alright, I'll look into it. > > Thanks. I am OK if we do not use my simplified version, but I think > there were some issues I noted with your last version. Yup, even some automated tool noticed a new leak ;-)
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
On Wed, Sep 21, 2016 at 07:45:50PM +0200, Kevin Daudt wrote: > On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote: > > Kevin Daudtwrites: > > > > > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote: > > >> > > >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits > > >> - mailinfo: unescape quoted-pair in header fields > > >> - t5100-mailinfo: replace common path prefix with variable > > > > > > Is this good enough, or do you want me to look into the feedback from > > > jeff? > > > > If you are talking about the simplified loop that deliberately sets > > a rule that is looser than RFC, yes, I'd like to see you at least > > consider the pros and cons of his approach, which looked nicer to my > > brief reading of it. > > > > It is perfectly OK by me (it may not be so if you ask Peff) if you > > decide that your version is better after doing so, though. > > Alright, I'll look into it. Thanks. I am OK if we do not use my simplified version, but I think there were some issues I noted with your last version. -Peff
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
Hi Junio, On Wed, 21 Sep 2016, Junio C Hamano wrote: > Johannes Schindelinwrites: > > >> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit > >> (merged to 'next' on 2016-08-14 at 6891bcd) > >> + rebase-interactive: drop early check for valid ident > >> > >> Even when "git pull --rebase=preserve" (and the underlying "git > >> rebase --preserve") can complete without creating any new commit > >> (i.e. fast-forwards), it still insisted on having a usable ident > >> information (read: user.email is set correctly), which was less > >> than nice. As the underlying commands used inside "git rebase" > >> would fail with a more meaningful error message and advice text > >> when the bogus ident matters, this extra check was removed. > >> > >> Will hold to see if people scream. > >> cf. <20160729224944.ga23...@sigill.intra.peff.net> > > > > Let's do this. > > We have already been doing it (i.e. "hold to see if people scream") > for some time. I meant: let's merge this to `master`. > Does it conflict with your effort to reimplement "rebase -i" in C I do not think so. > to keep this in 'next'? Do you want it to move to 'master'? I was > under the impression that it would not make a difference to have or not > have this patch once your reimplementation gets merged (meaning: the > removal of the three lines will be done by wholesale removal of > git-rebase--interactive.sh done the endgame of your series), so... Oh, I failed to make clear that my patch series do *not* remove git-rebase--interactive.sh. I just barely started to work to that end. While the speed improvements are quite noticable, the rebase--helper command still only implements the performance-critical code paths in C. There is quite a bit of work left to do before git-rebase--interactive.sh can be retired: - --root is not handled via the sequencer yet, - --preserve-merges is not handled either [*1*], - the shell script still sets up the state directory, - option parsing is still all-shell, - probably more tasks I forgot. The good news is that these parts can be converted independently from each other, and even by independent developers (hint, hint ;-)). Ciao, Dscho Footnote *1*: I am not sure that I want to port -p to C: in my view, this is a failed experiment, to be replaced with a design based on my Git garden shears. I tend to think that that part should be moved to a new shell script ("git-rebase--preserve-merges.sh"?) unless some developer other than me feels strongly enough to put their money where their mouth is and teach the sequencer about it.
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote: > Kevin Daudtwrites: > > > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote: > >> > >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits > >> - mailinfo: unescape quoted-pair in header fields > >> - t5100-mailinfo: replace common path prefix with variable > > > > Is this good enough, or do you want me to look into the feedback from > > jeff? > > If you are talking about the simplified loop that deliberately sets > a rule that is looser than RFC, yes, I'd like to see you at least > consider the pros and cons of his approach, which looked nicer to my > brief reading of it. > > It is perfectly OK by me (it may not be so if you ask Peff) if you > decide that your version is better after doing so, though. > > Thanks. Alright, I'll look into it.
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
Kevin Daudtwrites: > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote: >> >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits >> - mailinfo: unescape quoted-pair in header fields >> - t5100-mailinfo: replace common path prefix with variable > > Is this good enough, or do you want me to look into the feedback from > jeff? If you are talking about the simplified loop that deliberately sets a rule that is looser than RFC, yes, I'd like to see you at least consider the pros and cons of his approach, which looked nicer to my brief reading of it. It is perfectly OK by me (it may not be so if you ask Peff) if you decide that your version is better after doing so, though. Thanks.
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote: > > * kd/mailinfo-quoted-string (2016-09-19) 2 commits > - mailinfo: unescape quoted-pair in header fields > - t5100-mailinfo: replace common path prefix with variable Is this good enough, or do you want me to look into the feedback from jeff?
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
Duy Nguyenwrites: > On Tue, Sep 20, 2016 at 6:30 AM, Junio C Hamano wrote: >> * nd/checkout-disambiguation (2016-09-09) 4 commits >> - fixup! checkout.txt: document a common case that ignores ambiguation rules >> - checkout: fix ambiguity check in subdir >> - checkout.txt: document a common case that ignores ambiguation rules >> - checkout: add some spaces between code and comment >> >> "git checkout " does not follow the usual disambiguation >> rules when the can be both a rev and a path, to allow >> checking out a branch 'foo' in a project that happens to have a >> file 'foo' in the working tree without having to disambiguate. >> This was poorly documented and the check was incorrect when the >> command was run from a subdirectory. >> >> Waiting for an Ack for fixup! > > Oops, I didn't know (I have about 300 unread git mails in my inbox), Ack. Thanks.
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
Johannes Schindelinwrites: >> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit >> (merged to 'next' on 2016-08-14 at 6891bcd) >> + rebase-interactive: drop early check for valid ident >> >> Even when "git pull --rebase=preserve" (and the underlying "git >> rebase --preserve") can complete without creating any new commit >> (i.e. fast-forwards), it still insisted on having a usable ident >> information (read: user.email is set correctly), which was less >> than nice. As the underlying commands used inside "git rebase" >> would fail with a more meaningful error message and advice text >> when the bogus ident matters, this extra check was removed. >> >> Will hold to see if people scream. >> cf. <20160729224944.ga23...@sigill.intra.peff.net> > > Let's do this. We have already been doing it (i.e. "hold to see if people scream") for some time. Does it conflict with your effort to reimplement "rebase -i" in C to keep this in 'next'? Do you want it to move to 'master'? I was under the impression that it would not make a difference to have or not have this patch once your reimplementation gets merged (meaning: the removal of the three lines will be done by wholesale removal of git-rebase--interactive.sh done the endgame of your series), so...
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
On Tue, Sep 20, 2016 at 6:30 AM, Junio C Hamanowrote: > * nd/checkout-disambiguation (2016-09-09) 4 commits > - fixup! checkout.txt: document a common case that ignores ambiguation rules > - checkout: fix ambiguity check in subdir > - checkout.txt: document a common case that ignores ambiguation rules > - checkout: add some spaces between code and comment > > "git checkout " does not follow the usual disambiguation > rules when the can be both a rev and a path, to allow > checking out a branch 'foo' in a project that happens to have a > file 'foo' in the working tree without having to disambiguate. > This was poorly documented and the check was incorrect when the > command was run from a subdirectory. > > Waiting for an Ack for fixup! Oops, I didn't know (I have about 300 unread git mails in my inbox), Ack. -- Duy
Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
Hi, On Mon, 19 Sep 2016, Junio C Hamano wrote: > * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit > (merged to 'next' on 2016-08-14 at 6891bcd) > + rebase-interactive: drop early check for valid ident > > Even when "git pull --rebase=preserve" (and the underlying "git > rebase --preserve") can complete without creating any new commit > (i.e. fast-forwards), it still insisted on having a usable ident > information (read: user.email is set correctly), which was less > than nice. As the underlying commands used inside "git rebase" > would fail with a more meaningful error message and advice text > when the bogus ident matters, this extra check was removed. > > Will hold to see if people scream. > cf. <20160729224944.ga23...@sigill.intra.peff.net> Let's do this. Ciao, Dscho