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

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

 



Hi Peff,

On Wed, 3 Aug 2016, Jeff King wrote:

> 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.

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]