Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 4, 2016, 15:29 UTC
Message-ID: <alpine.DEB.2.20.1608041706040.5786@virtualbox>
In-Reply-To: <20160803165652.zek5df7tv5reg6w4@sigill.intra.peff.net>

Hi Peff,

On Wed, 3 Aug 2016, Jeff King wrote:

Show 17 quoted lines

> On Wed, Aug 03, 2016 at 09:53:18AM -0700, Junio C Hamano wrote:
> 
> > > Leaving aside Dscho's questions of whether pulling patches from email is
> > > convenient for most submitters (it certainly is for me, but I recognize
> > > that it is not for many), I would much rather see incremental fixup
> > > patches from you than whole "here's what I queued" responses.
> > 
> > Ah, yes, I misspoke.  It should be either an incremental diff or
> > in-line comment to spell out what got changed as a response to the
> > patch.
> > 
> > I find myself fixing the title the most often, which is part of the
> > "log message" you pointed out that would not convey well with the
> > "incremental diff" approach.
> 
> I mentioned a micro-format elsewhere in my message. And it certainly is
> nice to have something that can be applied in an automatic way.

Indeed. This is what I meant by my (succinct to the point of being intelligible, admittedly) reword! suggestion.

Let's clarify this idea.

I find myself using fixup! and squash! commits a lot. Actually, let me pull out the Linux key for that. I use those commits A LOT.

I know, I opposed the introduction of this feature initially (and I think that my concerns were nicely addressed by Junio's suggestion to guard this feature behind the --autosquash option). Guess what: I was wrong.

And I am really missing the same functionality for the commit message munging. These days, I find myself using `git commit --allow-empty --squash=$COMMIT -c $COMMIT` very often, duplicating the first line, adding an empty line between them, deleting the "squash! " prefix from the now-third line, and then editing the commit message as I want to. When it comes to cleaning up the branch via rebase -ki, I simply jump to the empty line after the squash! line and delete everything before it.

This is as repetitive, tedious and un-fun to me as having to transmogrify patches from the nice and cozy Git universe into the not-at-all compatible universe of mails (I congratulate you personally, Peff, for finding a mail client that works for you. I am still looking for one that does not suck, Alpine being the least sucky I settled for).

So my idea was to introduce a new --reword=<commit> option to `git commit` that would commit an empty patch and let the user edit the commit message, later replacing the original one with the new one. This is not *quite* as nice as I want it, because it makes the changes unobvious. On the other hand, I would not find a series of sed commands more obvious, in particular because that limits you in the ways of sed. And, you know, regexp. I like them, but I know many people cannot really work with them.

> But in practice, most review comments, for the commit message _or_ the
> text, are given in human-readable terms. And as a human, I read and
> apply them in sequence.

So true. I do the very same.

Show 6 quoted lines

> That pushes work onto the submitter, but saves work from the reviewers,
> who can quickly say "something like this..." without having to worry
> about making a full change, formatting it as a diff, etc.
> 
> I do think that's the right time-tradeoff to be making, as we have more
> submitters than reviewers.

I agree that it is the right trade-off. TBH I was shocked when I learned how much effort Junio puts into applying my patches. I do not want that. I want my branch to reflect pretty precisely (modulo sign-off, of course) what is going to be integrated into Git's source code.

I'd much prefer to resubmit a cleaned-up version, even if it was just the commit subjects, and be certain that `pu` and my branch are on the same page.

Instead, Junio puts in a tremendous amount of work, and it does not help anybody, because the local branches *still* do not have his fixups, and as a consequence subsequent iterations of the patch series will have to be fixed up *again*.

Just compare https://github.com/git/git/compare/1fd7e78...6999bc7 to https://github.com/dscho/git/compare/f8f7adc...3b4494c (the onelines are enough to show you just how different things are).

I'd much prefer the contributor (me, in this case) to put in a little more work, and have things consistent. And avoid unnecessary work on both sides.

Ciao, Dscho