On Mon, Oct 30, 2023 at 12:50 PM Bharath Rupireddy
wrote:
>
> Hi,
>
> How about adding code indent checks (like what BF member koel has) to
> the SanityCheck CI task? This helps catch indentation problems way
> before things are committed so that developers can find them out in
> their respective CI runs and lets developers learn the postgres code
> indentation stuff. It also saves committers time - git log | grep
> 'koel' | wc -l gives me 11 commits and git log | grep 'indentation' |
> wc -l gives me 97. Most, if not all of these commits went into fixing
> code indentation problems that could have been reported a bit early
> and fixed by developers/patch authors themselves.
>
> Thoughts?
Currently some of the code indentation issues that pgindent reports
are caught after the code gets committed either via the buildfarm
member koel or when someone runs pgindent before the code release.
These indentation issues are then fixed mostly by the committers in
separate commits. This is taking away the development time and causing
back and forth emails in mailing lists.
As of this writing, git log | grep 'koel' | wc -l gives 13 commits and
git log | grep 'indentation' | wc -l gives 100 commits (all may not be
related to code indentation per se). Almost all of these commits went
into fixing code indentation problems that could have been reported a
bit early and fixed by developers/patch authors themselves.
The attached patch adds a new cirrus-ci task with minimal resources
(with 1 CPU and ccache 150MB) that fails when pgindent is not happy
with any of the changes. This helps catch code indentation issues in
development phase way before things get committed. This step can kick
in developers cirrus-ci runs in their own accounts if cirrus-ci is
enabled in their development repos. Otherwise, it can also be enabled
to kick in cfbot runs (I've not explored this yet).
If we don't want this new task to drain the free credits/run time that
cirrus-ci offers, one possible way is to cook the code indentation
check into either SanityCheck or CompilerWarnings task to save on the
resources. If at all an indentation check like this is needed, we can
think of adding pgperltidy check as well.
Here's a development branch to see the new task in action
https://github.com/BRupireddy2/postgres/tree/add_code_indentation_check_to_cirrus_ci
- an intentional pgindent failure is detected by the new task where
the diff is uploaded as an artifact -
https://api.cirrus-ci.com/v1/artifact/task/6127561344811008/indentation/pgindent.diffs.
Thoughts?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
v1-0001-Add-code-indentation-check-to-cirrus-ci.patch
Description: Binary data
