Re: alter_table regression test problem

Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-11-07 06:23:13 -0800, Kevin Grittner wrote:

>>   I think this is one of those cases where the large
>> chunk of code inside the new "else" block should not be indented
>> with the initial patch -- a pgindent-based whitespace-only patch
>> should follow so that the substantive changes here are easier to
>> see in the initial patch.
>
> I think commiting code with fundamentally broken indentation like that
> is pretty much pointless though. Somebody who wants to look at the
> actual changes without the whitespace noise can just use git diff -w/git
> blame -w/... to eliminate those while viewing.

I think it is pretty much SOP for committers to prefer a patch that
adds a new pair of braces around 50 lines of code and only changes
non-whitespace of a few lines within that block to leave the block
at its old indentation.  It's up to the committer whether to indent
after review and make both substantive and whitespace changes
together, but I have definitely seen those done separately, or even
left for the next global pgindent run to fix.

Personally, I was surprised how small this apparently-large patch
became when I used git --color-words to compare it, which would be
another option, I guess; but there is precedent for the approach I
suggested.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company