From: Stephen Frost <sfrost@snowman.net>
To: Magnus Hagander <magnus@hagander.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Noah Misch <noah@leadboat.com>
Cc: Jesse Zhang <sbjesse@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: run pgindent on a regular basis / scripted manner
Date: Thu, 13 Aug 2020 12:30:44 -0400
Message-ID: <20200813163043.GZ29590@tamriel.snowman.net> (raw)
In-Reply-To: <CABUevEwg3LmBk-+ZiAYZW71dsB=boS-8sdYBux7M6qZp46avtA@mail.gmail.com>
References: <20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de>
<CAGf+fX71Oa2vzg0xdj+vgrjmdVrm=kRLc7H3SEGtCHkGhoWDCg@mail.gmail.com>
<328084.1597276021@sss.pgh.pa.us>
<20200813035308.GA1137170@rfd.leadboat.com>
<397020.1597291716@sss.pgh.pa.us>
<CABUevEwg3LmBk-+ZiAYZW71dsB=boS-8sdYBux7M6qZp46avtA@mail.gmail.com>
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
Attachments:
signature.asc (application/pgp-signature, 818B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, magnus@hagander.net, tgl@sss.pgh.pa.us, noah@leadboat.com, sbjesse@gmail.com, andres@anarazel.de
Subject: Re: run pgindent on a regular basis / scripted manner
In-Reply-To: <20200813163043.GZ29590@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox