Re: run pgindent on a regular basis / scripted manner

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, Jesse Zhang <sbjesse(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: run pgindent on a regular basis / scripted manner
Date: 2020-08-13 16:30:44
Message-ID: 20200813163043.GZ29590@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> There's another option here as well, that is a bit "softer", is to use a
> pre-commit hook.

Yeah, +1 on a pre-commit idea to help address this. I certainly agree
with Andres that it's quite annoying to deal with commits coming in that
aren't indented properly but are in some file that I'm working on.

> 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.

Yeah, no guarantee, but definitely seems like it'd be a good
improvement.

> > 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.

Yeah, I dislike (2) a lot too.

> 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.

Agreed, automated commits seems terribly risky.

> 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.

So, in our world, wouldn't this translate to 'make cfbot complain'?

I'm definitely a fan of the idea of having cfbot flag these and then we
maybe get to a point where it's not the committers dealing with fixing
patches that weren't pgindent'd properly, it's the actual patch
submitters being nagged about it...

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-08-13 16:45:08 Re: Parallel query hangs after a smart shutdown is issued
Previous Message Alvaro Herrera 2020-08-13 15:45:52 Re: Switch to multi-inserts for pg_depend