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