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-ayl...@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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
