Re: Commitfest problems

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Mark Cave-Ayland <mark(dot)cave-ayland(at)ilande(dot)co(dot)uk>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Josh Berkus <josh(at)agliodbs(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Joe Conway <mail(at)joeconway(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Commitfest problems
Date: 2014-12-16 07:55:39
Message-ID: 548FE57B.600@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12/16/2014 03:08 AM, 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.

Absolutely. I think that'd be awful.

I'm a fan of fairly fine grained patch series, but only where each patch
makes some sense by its self and doesn't break the tree. Splitting stuff
up purely for the sake of it or having patches that are non-working
intermediate states is painful and pointless.

Occasoinally it can be worth having a patch that introduces intermediate
compatibility code that's removed later, especially when it's small or
very self-contained. Most of the time I'm inclined to think that's not
worth it and it can create annoying noise in the history.

I'm only advocating it where the individual parts make a reasonable
amount of sense standing alone, and actually work by themselves.

Anyway, I'm not contributing much to the real topic here, which is
commitfest process issues. It's probably getting toward time for me to
butt out.

> 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.

Yup ... and with 'git am' and then 'git diff' on subtrees, it's pretty
convenient.

It's even easier when someone pushes work to GitHub working trees, so
you don't have to mess about with applying the changes, but that's a
trivial and unimportant convenience.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2014-12-16 08:23:39 Re: NUMERIC private methods?
Previous Message Jaime Casanova 2014-12-16 07:43:14 Re: TABLESAMPLE patch