Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Hi Jonathan, On Tue, 28 Aug 2018, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > On Mon, 27 Aug 2018, Junio C Hamano wrote: > >> Johannes Schindelin writes: > >>> Jonathan Nieder wrote: > > Please include this information in the commit message. It's super > helpful to find this kind of information about why a patch does what > it does when encountering a patch later "in the wild" (in git log -S > output). > [...] > >> I think what Jonathan finds helpful is the other half of the story > > > > I will await Jonathan's clarification. > > Junio's understanding is correct. > > More generally, I greatly appreciate the kind of motivation and > backstory that you write in cover letters, and I wish that more of > that would find its way into the commit messages instead. Really I > wish (and don't take this the wrong way --- I am not asking you to > write it unless it's your own itch) that GitGitGadget would put the > cover letter in single-patch series after the "---" line in the > individual patches, since that would make it easier for reviewers to > point out what content from the cover letter would be useful to add to > the commit message. > > That said, this is minor and not a reason to reroll this patch. It was > more that I wanted to give the hint for later patches. > > Thanks much and hope that helps, It does. I'll "rick-roll" a new iteration, as I just realized that (contrary to my recollection; I guess I'll need that vacation) the commit message is *seriously* lacking. I thought I had remembered that I copy-edited the commit message into the PR description. Clearly that was not the case. Thanks for the clarification that triggered my looking, Dscho
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Hi Junio, On Tue, 28 Aug 2018, Junio C Hamano wrote: > Johannes Schindelin writes: > > >> I do recall discouraging you from including irrelevant rant/whine in > >> the log message a few times in the recent past, and also I do recall > >> you never listening to me. Don't make me an excuse. > > > > Junio, I would really appreciate less emotional, and more professional > > conduct from you. > > Which part is unprofessional? > > Being caught and corrected with truth immediately after badmouthing > another by lying may hurt, but that is your problem, not mine. You did not catch me doing anything bad. You caught me telling Jonathan about having been criticized by you, for including background information *I* found relevant and *you* found irrelevant. And that was very important in this context, as he asked me to include something that I expected you to find irrelevant, too. Of course, confusingly, this time you found it relevant, even if it was as unrelated to the patch as in the previous case. So I am getting the impression that your critique was not actually professionally motivated, but very personal. And I do not appreciate that. I do not need to say anything more about this topic. Ciao, Dscho
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Johannes Schindelin wrote: > On Mon, 27 Aug 2018, Junio C Hamano wrote: >> Johannes Schindelin writes: >>> Jonathan Nieder wrote: Please include this information in the commit message. It's super helpful to find this kind of information about why a patch does what it does when encountering a patch later "in the wild" (in git log -S output). [...] >> I think what Jonathan finds helpful is the other half of the story > > I will await Jonathan's clarification. Junio's understanding is correct. More generally, I greatly appreciate the kind of motivation and backstory that you write in cover letters, and I wish that more of that would find its way into the commit messages instead. Really I wish (and don't take this the wrong way --- I am not asking you to write it unless it's your own itch) that GitGitGadget would put the cover letter in single-patch series after the "---" line in the individual patches, since that would make it easier for reviewers to point out what content from the cover letter would be useful to add to the commit message. That said, this is minor and not a reason to reroll this patch. It was more that I wanted to give the hint for later patches. Thanks much and hope that helps, Jonathan
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Johannes Schindelin writes: >> I do recall discouraging you from including irrelevant rant/whine in >> the log message a few times in the recent past, and also I do recall >> you never listening to me. Don't make me an excuse. > > Junio, I would really appreciate less emotional, and more professional > conduct from you. Which part is unprofessional? Being caught and corrected with truth immediately after badmouthing another by lying may hurt, but that is your problem, not mine.
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Hi Junio, On Mon, 27 Aug 2018, Junio C Hamano wrote: > Johannes Schindelin writes: > > >> Please include this information in the commit message. It's super > >> helpful to find this kind of information about why a patch does what > >> it does when encountering a patch later "in the wild" (in git log -S > >> output). > > > > I thought I did include the relevant part? As to the full back story: I > > was repeatedly dressed down by Junio in recent attempts to include more > > motivation in my commit messages. So I am reluctant to do as you say, > > because Junio is the BDFL here. > > I do recall discouraging you from including irrelevant rant/whine in > the log message a few times in the recent past, and also I do recall > you never listening to me. Don't make me an excuse. Junio, I would really appreciate less emotional, and more professional conduct from you. > I think what Jonathan finds helpful is the other half of the story I will await Jonathan's clarification. Ciao, Dscho > of what you did write in [1/1]. You wrote that it is no longer a > shell script and needs to follow a separate calling convention. > What was missing from that description that was given in [0/1] is > why the original "rebase-in-c" series was done while pretending that > the other effort "rebase-i-in-c" did not even exist, which made it > necessary to do this change as a separate step. > > And I tend to agree that it _is_ a relevant story in this case. > >
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Johannes Schindelin writes: >> Please include this information in the commit message. It's super >> helpful to find this kind of information about why a patch does what >> it does when encountering a patch later "in the wild" (in git log -S >> output). > > I thought I did include the relevant part? As to the full back story: I > was repeatedly dressed down by Junio in recent attempts to include more > motivation in my commit messages. So I am reluctant to do as you say, > because Junio is the BDFL here. I do recall discouraging you from including irrelevant rant/whine in the log message a few times in the recent past, and also I do recall you never listening to me. Don't make me an excuse. I think what Jonathan finds helpful is the other half of the story of what you did write in [1/1]. You wrote that it is no longer a shell script and needs to follow a separate calling convention. What was missing from that description that was given in [0/1] is why the original "rebase-in-c" series was done while pretending that the other effort "rebase-i-in-c" did not even exist, which made it necessary to do this change as a separate step. And I tend to agree that it _is_ a relevant story in this case.
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Hi Jonathan, On Wed, 22 Aug 2018, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > [nice description snipped] > > This patch fixes that. > > Please include this information in the commit message. It's super > helpful to find this kind of information about why a patch does what > it does when encountering a patch later "in the wild" (in git log -S > output). I thought I did include the relevant part? As to the full back story: I was repeatedly dressed down by Junio in recent attempts to include more motivation in my commit messages. So I am reluctant to do as you say, because Junio is the BDFL here. Ciao, Dscho
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
Hi, Johannes Schindelin wrote: [nice description snipped] > This patch fixes that. Please include this information in the commit message. It's super helpful to find this kind of information about why a patch does what it does when encountering a patch later "in the wild" (in git log -S output). Thanks, Jonathan
Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
"Johannes Schindelin via GitGitGadget" writes: > This patch fixes that. > > Note: while this patch targets pk/rebase-in-c-6-final, it will not work > correctly without ag/rebase-i-in-c. So my suggestion is to rewrite the > pk/rebas-in-c-6-final branch by first merging ag/rebase-i-in-c, then > applying this here patch, and only then cherry-pick "rebase: default to > using the builtin rebase". Yup, that sounds like a very sensible structure.
[PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase
The builtin rebase and the builtin interactive rebase have been developed independently, on purpose: Google Summer of Code rules specifically state that students have to work on independent projects, they cannot collaborate on the same project. The reason is probably the very fine tradition in academia to prohibit teamwork, which makes grading easier (at the expense of not exactly preparing the students for the real world, unless they want to stay in academia). One fallout is that the rebase-in-c and rebase-i-in-c patches cause no merge conflicts but a royal number of tests in the test suite to fail. It is easy to explain why: rebase-in-c was developed under the assumption that all rebase backends are implemented in Unix shell script and can be sourced via . git-rebase--, which is no longer true with rebase-i-in-c, where git-rebase--interactive is a hard-linked builtin. This patch fixes that. Note: while this patch targets pk/rebase-in-c-6-final, it will not work correctly without ag/rebase-i-in-c. So my suggestion is to rewrite the pk/rebas-in-c-6-final branch by first merging ag/rebase-i-in-c, then applying this here patch, and only then cherry-pick "rebase: default to using the builtin rebase". Johannes Schindelin (1): builtin rebase: prepare for builtin rebase -i builtin/rebase.c | 81 1 file changed, 81 insertions(+) base-commit: a5bb2345d2d414aba04e18531de1e0f041f0434a Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-23%2Fdscho%2Frebase-in-c-6-final-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-23/dscho/rebase-in-c-6-final-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/23 -- gitgitgadget
