On Sun, Dec 14, 2014 at 9:05 AM, Tom Lane <tgl@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
