Re: Run pgindent now? - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Run pgindent now?
Date
Msg-id CA+TgmobV3GodA1C_WW2t26EHQTv_jqwKc7+ujziLgKRw3C2AiQ@mail.gmail.com
Whole thread Raw
In response to Re: Run pgindent now?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Run pgindent now?
Re: Run pgindent now?
Re: Run pgindent now?
List pgsql-hackers
On Mon, May 25, 2015 at 5:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>> On Mon, May 25, 2015 at 04:52:38PM -0300, Alvaro Herrera wrote:
>>> Something is wrong.  See aclchk.c changes.
>
>> Yes, this is what I was concerned about.  "aclitem" was a typedef in 9.0
>> and 9.1, and the use of that as a typedef in 9.4 is certainly odd:
>
>>       -       aclitem.ai_grantor = grantorId;
>>       +       aclitem.    ai_grantor = grantorId;
>
> Yeah.  I think we might've gotten rid of that typedef partially in order
> to fix this.
>
> A different strategy we could consider is "use HEAD's typedef list
> even in the back branches".  This would in some situations lead to
> inferior-looking results in the back branches, but that's probably better
> than inferior results in HEAD.  (In any case, we want the same typedef
> list across all branches.  Then anyplace where the results diverge, there
> must have been non-pgindent code changes, so that back-patching would
> require manual fixups anyway.)

This is kind of why I think that reindenting the back branches is
unlikely to be productive: it only helps if you can get pgindent to do
the same thing on all branches, and I bet that's going to be tough.

Realistically, with merge.conflictstyle = diff3 (why is this not the
default?), resolving whitespace conflicts that occur when you try to
cherry-pick is typically not very difficult.  But every time we
pgindent, especially with slightly different settings, we cause tools
like 'git blame' to return less useful answers.  And that sucks.

We also risk breaking private patchsets that people are carrying - of
course, Advanced Server is one (very large) such patchset, but it's
hardly the only place where people are compiling a non-standard
distribution that has to be reconciled with upstream changes.

I'm not going to put up a huge fuss if we decide to go ahead with
this, but I still think it's a bad plan, especially with regarding to
existing branches that have not been re-indented in years.  I bet it
won't save that much back-patching effort - maybe not any, on net -
and I bet it will inconvenience users and developers in various subtle
ways that we may not even hear about but which are still quite real.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: ERROR: MultiXactId xxxx has not been created yet -- apparent wraparound
Next
From: Josh Berkus
Date:
Subject: Re: ERROR: MultiXactId xxxx has not been created yet -- apparent wraparound