Re: Commitfest problems - Mailing list pgsql-hackers

On 15/12/14 19:24, Andrew Dunstan wrote:

> On 12/15/2014 02:08 PM, Robert Haas wrote:
>> On Sun, Dec 14, 2014 at 2:24 PM, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>> However if it were posted as part of patchset with a subject of "[PATCH]
>>> gram.y: add EXCLUDED pseudo-alias to bison grammar" then this may pique
>>> my interest enough to review the changes to the grammar rules, with the
>>> barrier for entry reduced to understanding just the PostgreSQL-specific
>>> parts.
>> Meh.  Such a patch couldn't possibly compile or work without modifying
>> other parts of the tree.  And I'm emphatically opposed to splitting a
>> commit that would have taken the tree from one working state to
>> another working state into a series of patches that would have taken
>> it from a working state through various non-working states and
>> eventually another working state.  Furthermore, if you really want to
>> review that specific part of the patch, just look for what changed in
>> gram.y, perhaps by applying the patch and doing git diff
>> src/backend/parser/gram.y.  This is really not hard.
>>
>> I certainly agree that there are cases where patch authors could and
>> should put more work into decomposing large patches into bite-sized
>> chunks.  But I don't think that's always possible, and I don't think
>> that, for example, applying BRIN in N separate pieces would have been
>> a step forward.  It's one feature, not many.
>>
> 
> +1
> 
> I have in the past found the "bunch of patches" approach to be more that
> somewhat troublesome, especially when one gets "here is an update to
> patch nn of the series" and one has to try to compose a coherent set of
> patches in order to test them. A case in point I recall was the original
> Foreign Data Wrapper patchset.

In practice, people don't tend to post updates to individual patches in
that way. What happens is that after a week or so of reviews, people go
away and rework the patch and come back with a complete updated patchset
clearly marked as [PATCHv2] with the same subject line and an updated
cover letter describing the changes, so a complete coherent patchset is
always available.

Now as I mentioned previously, one of the disadvantages of splitting
patches in this way is that mailing list volume tends to grow quite
quickly - hence the use of [PATCH] filters and system identifiers in the
subject line to help mitigate this.

Whilst the volume of mail was a shock to begin with, having stuck with
it for a while I personally find that the benefits to development
outweigh the costs. Each individual project is different, but I believe
that there are good ideas here that can be used to improve workflow,
particularly when it comes to patch review.


ATB,

Mark.