Re: run pgindent on a regular basis / scripted manner

On Thu, Aug 13, 2020 at 6:08 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Noah Misch <noah(at)leadboat(dot)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.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>