Horton
Menu

Re: run pgindent on a regular basis / scripted manner

Stephen Frost <sfrost@snowman.net>

View thread
From: Stephen Frost <sfrost@snowman.net>
To: Magnus Hagander <magnus@hagander.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Noah Misch <noah@leadboat.com>, Jesse Zhang <sbjesse@gmail.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-08-13T16:30:44Z
Lists: pgsql-hackers 
Greetings,

* Magnus Hagander (magnus@hagander.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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix comment from commit 22655aa231.

  2. Add a few recent commits to .git-blame-ignore-revs.

  3. Pre-beta2 mechanical code beautification.

  4. Pre-beta mechanical code beautification.

  5. Make agreed-on updates in perltidy options.

  6. Remove obsolete pgindent options --code-base and --build

  7. Integrate pg_bsd_indent into our build/test infrastructure.

  8. Sync pg_bsd_indent's copyright notices with Postgres practice.

  9. Import pg_bsd_indent sources.

  10. pgindent: filter files for the --commit option

  11. pgindent: more ways to find files to indent

  12. Fix pgindent --show-diff option.

  13. Add non-destructive modes to pgindent

  14. Initial pgindent run with pg_bsd_indent version 2.0.