Re: run pgindent on a regular basis / scripted manner - Mailing list pgsql-hackers
| From | Magnus Hagander |
|---|---|
| Subject | Re: run pgindent on a regular basis / scripted manner |
| Date | |
| Msg-id | CABUevEwg3LmBk-+ZiAYZW71dsB=boS-8sdYBux7M6qZp46avtA@mail.gmail.com Whole thread Raw |
| In response to | Re: run pgindent on a regular basis / scripted manner (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: run pgindent on a regular basis / scripted manner
|
| List | pgsql-hackers |
On Thu, Aug 13, 2020 at 6:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Noah Misch <noah@leadboat.com> writes:
> On Wed, Aug 12, 2020 at 07:47:01PM -0400, Tom Lane wrote:
>> If the workflow is commit first and re-indent later, then we can always
>> revert the pgindent commit and clean things up manually; but an auto
>> re-indent during commit wouldn't provide that history.
> There are competing implementations of assuring pgindent-cleanliness at every
> check-in:
> 1. After each push, an automated followup commit appears, restoring
> pgindent-cleanliness.
> 2. "git push" results in a commit that melds pgindent changes into what the
> committer tried to push.
> 3. "git push" fails, for the master branch, if the pushed commit disrupts
> pgindent-cleanliness.
There's another option here as well, that is a bit "softer", is to use a pre-commit hook.
That is, it's a hook that runs on the committers machine prior to the commit. This hook can then yell "hey, you need to run pgindent before committing this", but it gives the committer the ability to do --no-verify and commit anyway (thus won't block things that are urgent).
Since it allows a simple bypass, and very much relies on the committer to remember to install the hook in their local repository, this is not a guarantee in any way. So it might need to be done together with something else in the background doing like a daily job or so, but it might make that background work be smaller and fewer changes.
This obviously only works in the case where we can rely on the committers to remember to install such a hook, but given the few committers that we do have, I think we can certainly get that up to an "acceptable rate" fairly easily. FWIW, this is similar to what we do in the pgweb, pgeu and a few other repositories, to ensure python styleguides are followed.
> Were you thinking of (2)?
I was objecting to (2). (1) would perhaps work. (3) could be pretty
darn annoying, especially if it blocks a time-critical security patch.
FWIW, I agree that (2) seems like a really bad option. In that suddenly a committer has committed something they were not aware of.
> Regarding typedefs.list, I would use the in-tree one, like you discussed here:
Yeah, after thinking about that more, it seems like automated
typedefs.list updates would be far riskier than automated reindent
based on the existing typedefs.list. The latter could at least be
expected not to change code unrelated to the immediate commit.
typedefs.list updates need some amount of adult supervision.
(I'd still vote for nag-mail to the committer whose work got reindented,
in case the bot made things a lot worse.)
Yeah, I'm definitely not a big fan of automated commits, regardless of if it's just indent or both indent+typedef. It's happened at least once, and I think more than once, that we've had to basically hard reset the upstream repository and clean things up after automated commits have gone bonkers (hi, Bruce!). Having an automated system do the whole flow of change->commit->push definitely invites this type of problem.
There are many solutions that do such things but that do it in the "github workflow" way, which is they do change -> commit -> create pull request, and then somebody eyeballs that pullrequest and approves it. We don't have PRs, but we could either have a script that simply sends out a patch to a mailinglist, or we could have a script that maintains a separate branch which is auto-pgindented, and then have a committer squash-merge that branch after having reviewed it.
Maybe something like that in combination with a pre-commit hook per above.
--
pgsql-hackers by date: