Feedback-ID: i131946ab:Fastmail
Message-ID: <764a8f96-58c3-361d-428f-7ef827d62023@enterprisedb.com>
Date: Mon, 6 Feb 2023 18:17:02 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.6.1
Subject: Re: run pgindent on a regular basis / scripted manner
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>, Noah Misch <noah@leadboat.com>
Cc: Bruce Momjian <bruce@momjian.us>, Jelte Fennema <postgres@jeltef.nl>,
 Andres Freund <andres@anarazel.de>, Peter Geoghegan <pg@bowt.ie>,
 Magnus Hagander <magnus@hagander.net>,
 Alvaro Herrera <alvherre@2ndquadrant.com>, Stephen Frost
 <sfrost@snowman.net>, Jesse Zhang <sbjesse@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
References: <20230123011002.fzcaa3krql3mqsfn@awork3.anarazel.de>
 <1509591.1674437376@sss.pgh.pa.us> <1608158.1674486546@sss.pgh.pa.us>
 <20230123173136.knn77pbuakhvjfgk@awork3.anarazel.de>
 <Y874Fh0q4mkk1s22@momjian.us>
 <CAGECzQTqoGHrePM_pwmhaM+J1XOygYjzFod4sf=PAt7J4A19Mw@mail.gmail.com>
 <2238493.1674572097@sss.pgh.pa.us> <Y9ArogF7c+UgPgq9@momjian.us>
 <20230129010603.GA1677550@rfd.leadboat.com> <Y9groc0Nyy7SOROy@momjian.us>
 <20230202062838.GA1832088@rfd.leadboat.com>
 <1477791.1675320029@sss.pgh.pa.us>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <1477791.1675320029@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/764a8f96-58c3-361d-428f-7ef827d62023%40enterprisedb.com>
Precedence: bulk

On 02.02.23 07:40, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
>> Regarding the concern about a pre-receive hook blocking an emergency push, the
>> hook could approve every push where a string like "pgindent: no" appears in a
>> commit message within the push.  You'd still want to make the tree clean
>> sometime the same week or so.  It's cheap to provide a break-glass like that.
> 
> I think the real question here is whether we can get all (or at least
> a solid majority of) committers to accept such draconian constraints.
> I'd buy into it, and evidently so would you, but I can't help noting
> that less than a quarter of active committers have bothered to
> comment on this thread.  I suspect the other three-quarters would
> be quite annoyed if we tried to institute such requirements.  That's
> not manpower we can afford to drive away.

I have some concerns about this.

First, as a matter of principle, it would introduce another level of 
gatekeeping power.  Right now, the committers are as a group in charge 
of what gets into the tree.  Adding commit hooks that are installed 
somewhere(?) by someone(?) and can only be seen by some(?) would upset 
that.  If we were using something like github or gitlab (not suggesting 
that, but for illustration), then you could put this kind of thing under 
.github/ or similar and then it would be under the same control as the 
source code itself.

Also, pgindent takes tens of seconds to run, so hooking that into the 
git push process would slow this down quite a bit.  And maybe we want to 
add pgperltidy and so on, where would this lead?  If somehow your local 
indenting doesn't give you the "correct" result for some reason, you 
might sit there for minutes and minutes trying to fix and push and fix 
and push.

Then, consider the typedefs issue.  If you add a typedef but don't add 
it to the typedefs list but otherwise pgindent your code perfectly, the 
push would be accepted.  If then later someone updates the typedefs 
list, perhaps from the build farm, it would then reject the indentation 
of your previously committed code, thus making it their problem.

I think a better way to address these issues would be making this into a 
test suite, so that you can run some command that checks "is everything 
indented correctly".  Then you can run this locally, on the build farm, 
in the cfbot etc. in a uniform way and apply the existing 
blaming/encouragement processes like for any other test failure.