Re: Commitfest problems - Mailing list pgsql-hackers

On 14/12/14 17:05, Tom Lane wrote:

> Craig Ringer <craig@2ndquadrant.com> writes:
>> On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote:
>>> Compare this to say, for example, huge patches such as RLS.
> 
>> I specifically objected to that being flattened into a single monster
>> patch when I saw that'd been done. If you look at my part in the work on
>> the row security patch, while I was ultimately unsuccessful in getting
>> the patch mergeable I spent quite a bit of time splitting it up into a
>> logical patch-series for sane review and development. I am quite annoyed
>> that it was simply flattened back into an unreviewable, hard-to-follow
>> blob and committed in that form.
> 
> TBH, I'm not really on board with this line of argument.  I don't find
> broken-down patches to be particularly useful for review purposes.  An
> example I was just fooling with this week is the GROUPING SETS patch,
> which was broken into three sections for no good reason at all.  (The
> fourth and fifth subpatches, being alternative solutions to one problem,
> are in a different category of course.)  Too often, decisions made in
> one subpatch don't make any sense until you see the larger picture.

The way in which this is normally handled is via the cover letter which
is going to be nicely captured in the mail archives; typically a cover
letter for a patchset consists of several paragraphs along the lines of
patches 1-3 do some re-org work, patch 4 reworks the Y API for new
feature X implemented in patches 9-12.

As an example take a look at the cover letter for this patch submitted
over the past few days:
http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg01990.html. It
seems to me that this would give the level of detail which you are
looking for.

I agree that definitely in some cases patches could be broken into "too
fine" pieces, but then I think it's perfectly okay for committers to
turn around and request the series be re-submitted in more sensible
chunks if they feel things have gone too far the other way.

> Also, speaking of the larger picture: the current Postgres revision
> history amounts to 37578 commits (as of sometime yesterday) --- and that's
> just in the HEAD branch.  If we'd made an effort to break feature patches
> into bite-size chunks like you're recommending here, we'd probably have
> easily half a million commits in the mainline history.  That would not be
> convenient to work with, and I really doubt that it would be more useful
> for "git bisect" purposes, and I'll bet a large amount of money that most
> of them would not have had commit messages composed with any care at all.

Having worked on a few kernel patches myself, git is surprisingly
effective on huge repositories once the cache is warmed up so I don't
feel this would be an issue with a PostgreSQL git repository which will
be many magnitudes smaller.

In terms of commit messages, I don't know if you missed that part of my
original response to Craig but it's considered normal for a maintainer
to reject a patch because of an incorrect/irrelevant commit message.

So if I submitted a patch that fixed a particular bug but you as a
committer weren't happy with the commit message, then I'm perfectly okay
with you asking me to resubmit with updated/corrected patch message wording.


ATB,

Mark.