On Mon, Oct 30, 2023 at 12:50 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.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
v1-0001-Add-code-indentation-check-to-cirrus-ci.patch
Description: Binary data
