Subject: Three commit tips

From: Bruce Momjian <[email-masked]>
Date: Thu, 2 Nov 2023 11:22:38 -0400

I have three suggestions on committing that I thought would be helpful
to the hacker audience.

First, I have been hesitant to ascribe others as patch authors if I
heavily modified a doc patch because I didn't want them blamed for any
mistakes I made.  However, I also want to give them credit, so I decided
I would annotate commits with "partial", e.g.:

	Author:  Andy Jackson (partial)

Second, I have found the git diff option --word-diff=color to be very
helpful and I have started using it, especially for doc patches where
the old text is in red and the new text is in green.  Posting patches in
that format is probably not helpful though.

Third, I have come up with the following shell script to test for proper
pgindentation, which I run automatically before commit:

	# https://www.postgresql.org/message-id/CAGECzQQL-Dbb%2BYkid9Dhq-491MawHvi6hR_NGkhiDE%2B5zRZ6vQ%40mail.gmail.com
	src/tools/pgindent/pgindent $(git diff --name-only --diff-filter=ACMR) > /tmp/$$

	if [ \( "$(wc -l < /tmp/$$)" -eq 1 -a "$(expr match "$(cat /tmp/$$)" "No files to process\>")" -eq 0 \) -o \
	     "$(wc -l < /tmp/$$)" -gt 1 ]
	then	echo "pgindent failure in master branch, exiting." 1>&2
		cat /tmp/$$
		exit 1
	fi

-- 
  Bruce Momjian  <[email-masked]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

---

From: Nathan Bossart <[email-masked]>
Date: Thu, 2 Nov 2023 11:07:19 -0500

> 	Author:  Andy Jackson (partial)

I tend to use "Co-authored-by" for this purpose.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

---

From: Alexander Lakhin <[email-masked]>
Date: Fri, 3 Nov 2023 09:00:00 +0300

> pgindentation, which I run automatically before commit:

I would also suggest using a script like attached to check a patch for
new unicums (usually some of them are typos or inconsistencies introduced
by the patch).
The script itself can be improved, without a doubt, but it still can be
useful as-is. For example, for a couple of arbitrarily chosen today's
patches [1] it shows:

.../postgresql.git$ check-patch-for-unicums.sh .../v4-0001-Make-all-SLRU-buffer-sizes-configurable.patch
New unicums:
cotents:        ./doc/src/sgml/config.sgml:        Specifies the amount of memory to use to cache the cotents of
====

.../postgresql.git$ check-patch-for-unicums.sh .../v4-0003-Partition-wise-slru-locks.patch
New unicums:
bank_tranche_id: ./src/include/access/slru.h: int bank_tranche_id, SyncRequestHandler sync_handler);
CommitTSSLRU:   ./src/backend/storage/lmgr/lwlock.c: "CommitTSSLRU",
CommitTsSLRULock: ./src/backend/storage/lmgr/lwlocknames.txt:#38 was CommitTsSLRULock
ControlLock:    ./src/backend/replication/slot.c:        * flag while holding the ControlLock as otherwise a concurrent
ctllock:        ./src/backend/access/transam/slru.c: * ctllock: LWLock to use to control access to the shared control 
structure.
cur_lru_count:  ./src/backend/access/transam/slru.c:             * Notice that this next line forcibly advances 
cur_lru_count to a
...

[1] https://www.postgresql.org/message-id/CAFiTN-uyiUXU__VwJAimZ%2B6jQbm1s4sYi6u4fXBD%3D47xVd%3Dthg%40mail.gmail.com

Best regards,
Alexander