On Sun, Dec 14, 2014 at 9:05 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > 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.
It sounds like they didn't use the technique effectively, then. I think it will be useful to reviewers that I've broken out the mechanism through which the ON CONFLICT UPDATE patch accepts the EXCLUDED.* pseudo-alias into a separate commit (plus docs in a separate commit, as well as tests) - it's a non-trivial additional piece of code that it wouldn't be outrageous to omit in a release, and isn't particularly anticipated by earlier cumulative commits. Maybe people don't have a good enough sense of what sort of subdivision is appropriate yet. I think that better style will emerge over time. Of course, if you still prefer a monolithic commit, it's pretty easy to produce one from a patch series. It's not easy to go in the other direction, though. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
