Re: Commitfest problems

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(dot)cave-ayland(at)ilande(dot)co(dot)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.