On Wed, 18 Oct 2023 at 22:07, Jelte Fennema <postgres@jeltef.nl> wrote:
> But based on the contents of the fixup commits a commonality seems to
> be that the fixup only fixes a few lines, quite often touching only
> comments. So it seems like the main reason for breaking koel is
> forgetting to re-run pgindent after some final cleanup/wording
> changes/typo fixes. And that seems like an expected flaw of being
> human instead of a robot, which can only be worked around with better
> automation.
I wonder if you might just be assuming these were caused by
last-minute comment adjustments. I may have missed something on the
thread, but it could be that, or it could be due to the fact that
pgindent just simply does more adjustments to comments than it does
with code lines.
On Wed, 18 Oct 2023 at 06:40, David Rowley <dgrowleyml@gmail.com> wrote:
> > I agree that it's not nice to add yet another way of breaking the
> > buildfarm and even more so when the committer did make check-world
> > before committing. We have --enable-tap-tests, we could have
> > --enable-indent-checks and have pgindent check the code is correctly
> > indented during make check-world. Then just not have
> > --enable-indent-checks in CI.
>
> I think --enable-indent-checks sounds like a good improvement to the
> status quo. But I'm not confident that it will help remove the cases
> where only a comment needs to be re-indented. Do commiters really
> always run check-world again when only changing a typo in a comment? I
> know I probably wouldn't (or at least not always).
I can't speak for others, but I always make edits in a dev branch and
do "make check-world" before doing "git format-patch" before I "git
am" that patch into a clean repo. Before I push, I'll always run
"make check-world" again as sometimes master might have moved on a few
commits from where the dev branch was taken (perhaps I need to update
the expected output of some newly added EXPLAIN tests if say doing a
planner adjustment). I personally never adjust any code or comments
after the git am. I only sometimes adjust the commit message.
So, in theory at least, if --enable-indent-checks existed and I used
it, I shouldn't break koel... let's see if I just jinxed myself.
It would be good to learn how many of the committers out of the ones
you listed that --enable-indent-checks would have saved from breaking
koel.
David