Add code indentation check to cirrus-ci (was Re: Add BF member koel-like indentation checks to SanityCheck CI)

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Add code indentation check to cirrus-ci (was Re: Add BF member koel-like indentation checks to SanityCheck CI)
Date: 2023-12-08 11:39:31
Message-ID: CALj2ACVwbWD+_vhDrL2Lbue=qWyOmurP_Gi7NuAkfXPb+e6FRg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 30, 2023 at 12:50 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> 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

Attachment Content-Type Size
v1-0001-Add-code-indentation-check-to-cirrus-ci.patch application/octet-stream 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-12-08 11:44:38 Remove some unnecessary includes of "access/xlog_internal.h"
Previous Message Alvaro Herrera 2023-12-08 11:19:22 Re: [PATCH] minor reloption regression tests improvement