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
