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]

 



On Thu, Aug 04, 2016 at 05:29:52PM +0200, Johannes Schindelin wrote:

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

I don't have a real opinion on this. I probably wouldn't use it, but I
have no problem with it existing. I think it's somewhat orthogonal to
the idea of _transmitting_ those reword operations to somebody else.

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

Like you, I have occasionally been bitten by Junio doing a fixup, and
then I end up re-rolling, and lose that fixup (or have to deal with
porting it forward with awkward tools).

But I think such fixups are a calculated risk. Sometimes they save a lot
of time, both for the maintainer and the contributor, when they manage
to prevent another round-trip of the patch series to the list.

IOW, if the flow is something like:

  1. Contributor sends patches. People review.

  2. Minor fixups noticed by maintainer, fixed while applying.

  3. Only one small fixup needed from review. Contributor sends
     squashable patch. Maintainer squashes.

then I think that is a net win over sending the whole series again, for
the contributor (who does not bother sending), reviewers (who really
only need to look at the interdiff, which is what that squash is in the
first place), and the maintainer (who can squash just as easily as
re-applying the whole series).

It does mean the "final" version of the series is never on the list. It
has to be pieced together from the squash (and sometimes step 2 is not
even mentioned on-list).

So I think it is really a judgement call for step (3) on what is a
"small" fixup, and whether it is easier for everybody to look at the
squash interdiff and say "yep, that's right", versus re-reviewing the
whole series.

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

And that is the flip side. If the flow above does not happen, then step
2 just becomes a pain.

I don't have a silver bullet or anything. I'm mostly just musing.

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