Thread: Three commit tips
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 <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Thu, Nov 02, 2023 at 11:22:38AM -0400, Bruce Momjian wrote: > 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) I tend to use "Co-authored-by" for this purpose. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Thu, Nov 2, 2023 at 11:07:19AM -0500, Nathan Bossart wrote: > On Thu, Nov 02, 2023 at 11:22:38AM -0400, Bruce Momjian wrote: > > 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) > > I tend to use "Co-authored-by" for this purpose. Very good idea. I will use that instead. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
On Thu, Nov 2, 2023 at 8:52 PM Bruce Momjian <bruce@momjian.us> wrote: > > 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 > Looks useful. Git supports pre-push hook: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks, a sample script showing how to access the commits being pushed and other arguments at https://github.com/git/git/blob/87c86dd14abe8db7d00b0df5661ef8cf147a72a3/templates/hooks--pre-push.sample. I have not used it. But it seems that your script can be used to implement the pre-push hook. -- Best Wishes, Ashutosh Bapat
02.11.2023 18:22, Bruce Momjian wrote: > Third, I have come up with the following shell script to test for proper > 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