Hi,
On 2023-02-06 18:17:02 +0100, Peter Eisentraut wrote:
> First, as a matter of principle, it would introduce another level of
> gatekeeping power. Right now, the committers are as a group in charge of
> what gets into the tree. Adding commit hooks that are installed
> somewhere(?) by someone(?) and can only be seen by some(?) would upset that.
> If we were using something like github or gitlab (not suggesting that, but
> for illustration), then you could put this kind of thing under .github/ or
> similar and then it would be under the same control as the source code
> itself.
Well, we did talk about adding a pre-commit hook to the repository, with
instructions for how to enable it. And I don't see a problem with adding the
pre-receive we're discussing here to src/tools/something.
> Also, pgindent takes tens of seconds to run, so hooking that into the git
> push process would slow this down quite a bit. And maybe we want to add
> pgperltidy and so on, where would this lead?
Yes, I've been annoyed by this as well. This is painful, even without a push
hook. Not just for pgindent, headerscheck/cpluspluscheck are quite painful as
well. I came to the conclusion that we ought to integrate pgindent etc into
the buildsystem properly. Instead of running such targets serially across all
files, without logic to prevent re-processing files, the relevant targets
ought to be run once for each process, and create a stamp file.
> If somehow your local indenting doesn't give you the "correct" result for
> some reason, you might sit there for minutes and minutes trying to fix and
> push and fix and push.
I was imagining that such a pre-receive hook would spit out the target that
you'd need to run locally to verify that the issue is resolved.
> Then, consider the typedefs issue. If you add a typedef but don't add it to
> the typedefs list but otherwise pgindent your code perfectly, the push would
> be accepted. If then later someone updates the typedefs list, perhaps from
> the build farm, it would then reject the indentation of your previously
> committed code, thus making it their problem.
I'd like to address this one via the buildsystem as well. We can do the
trickery that the buildfarm uses to extract typedefs as part of the build, and
update typedefs.list with the additional types. There's really no need to
force us to do this manually.
Greetings,
Andres Freund