Re: Commitfest problems

On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote:

> If I could name just one thing that I think would improve things it
> would be submission of patches to the list in git format-patch format.
> Why? Because it enables two things: 1) by definition patches are
> "small-chunked" into individually reviewable pieces and 2) subject lines
> allow list members to filter things that aren't relevant to them.

Personally I do just that. Even though the official docs say context
diff is preferred, I think most people now use context diff in practice,
from 'git diff'.

I'm surprised most people don't use git format-patch .

Note that we can't just "git am" a patch from git format-patch at the
moment, because policy is that the committer should be the Author field.
The project would have to define a transition point where we started
using the git Author field for the author and the separate Committer
field, like git-am does by default. I'm not sure that'll ever actually
happen, or that it's even desirable.

> - Patchsets must be iterative and fully bisectable, with clear comments
> supplied in the commit message

Fully support this one.

Also in the smallest reasonable size divisions.

> - If a maintainer is happy with the whole patchset, they reply to the
> cover letter with a "Reviewed-by" and a line stating which subtree the
> patchset has been applied to. Maintainers add a "Signed-off-by" to all
> patches accepted via their subtree.

Sounds a lot like the kernel workflow (though in practice it seems like
the kernel folks tend bypass all this mailing list guff and just use
direct pulls/pushes for lots of things).

> - Any member of the mailing list may reply/review an individual patch by
> hitting "reply" in their mail client. Patch comments are included
> inline. If a reviewer is happy with an individual patch, they reply to
> the patch and add a "Reviewed-by" line; anyone can provide a review,
> even if they are not a maintainer

This is "fun" with many mail clients that like to linewrap text bodies
and don't permit inline replies to attached text.

> 6) Smaller patches are applied more often
>
> By breaking large patches down into smaller chunks, even if the main
> feature itself isn't ready to be applied to the main repository then a
> lot of the preceding patches laying down the groundwork can.

I think PostgreSQL does OK on smaller patches - at least sometimes.
There can be a great deal of bikeshedding and endless arguments,
back-and-forth about utility, in-tree users, etc, but no formal process
is ever going to change that. The process of getting an uncontroversial
patch into Pg is generally painless, if often rather slow unless a
committer sees it and commits it immediately upon it being posted.

> The benefits here are that people with large out-of-tree patches

I'm not sure we have all that many people in that category - though I'm
a part of a team that most certainly does fit the bill.

Even the "small" patches for BDR have been challenging and often
contentious though.

> Since as large features get
> implemented as a series of smaller patches

A big barrier here is the "we must have in-tree users" thing, which
makes it hard to do iterative work on a finer grained scale.

Some C-level unit tests that let us exercise small units of
functionality might ease those complaints. Right now the closest we have
to that is contrib/test_[blah] which is only useful for public API and
even then quite limited.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services