From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Jelte Fennema <postgres(at)jeltef(dot)nl> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, Peter Geoghegan <pg(at)bowt(dot)ie>, Magnus Hagander <magnus(at)hagander(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Noah Misch <noah(at)leadboat(dot)com>, Jesse Zhang <sbjesse(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: run pgindent on a regular basis / scripted manner |
Date: | 2023-01-24 14:54:57 |
Message-ID: | 2238493.1674572097@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Jelte Fennema <postgres(at)jeltef(dot)nl> writes:
>> One issue on requiring patches to have run pgindent previously is
>> actually the typedef list. If someone adds a typedef in a commit, they
>> will see different pgident output in the committed files, and perhaps
>> others, and the new typedefs might only appear after the commit, causing
>> later commits to not match.
> I'm not sure I understand the issue you're pointing out. If someone
> changes the typedef list, imho they want the formatting to change
> because of that. So requiring an addition to the typedef list to also
> commit reindentation to all files that this typedef indirectly impacts
> seems pretty reasonable to me.
I think the issue Bruce is pointing out is that this is another mechanism
whereby different people could get different indentation results.
I fear any policy that is based on an assumption that indentation has
One True Result is going to fail.
As a concrete example, suppose Alice commits some code that uses "foo"
as a variable name, and more or less concurrently, Bob commits something
that defines "foo" as a typedef. Bob's change is likely to have
side-effects on the formatting of Alice's code. If they're working in
well-separated parts of the source tree, nobody is likely to notice
that for awhile --- but whoever next touches the files Alice touched
will be in for a surprise, which will be more or less painful depending
on whether we've installed brittle processes.
As another example, the mechanisms we use to create the typedefs list
in the first place are pretty squishy/leaky: they depend on which
buildfarm animals are running the typedef-generation step, and on
whether anything's broken lately in that code --- which happens on
a fairly regular basis (eg [1]). Maybe that could be improved,
but I don't see an easy way to capture the set of system-defined
typedefs that are in use on platforms other than your own. I
definitely do not want to go over to hand maintenance of that list.
I think we need to be content with a "soft", it's more-or-less-right
approach to indentation. As I explained to somebody upthread, the
main benefit of this for most people is avoiding the need for a massive
once-a-year reindent run that causes merge failures for many pending
patches. But we don't need to completely eliminate such runs to get
99.9% of that benefit; we only need to reduce the number of places
they're likely to touch.
regards, tom lane
[1] https://github.com/PGBuildFarm/client-code/commit/dcca861554e90d6395c3c153317b0b0e3841f103
From | Date | Subject | |
---|---|---|---|
Next Message | Takamichi Osumi (Fujitsu) | 2023-01-24 14:57:22 | RE: Time delayed LR (WAS Re: logical replication restrictions) |
Previous Message | Bharath Rupireddy | 2023-01-24 14:46:18 | Re: Improve GetConfigOptionValues function |