Committing checklist

From PostgreSQL wiki
Jump to navigationJump to search

This document is an attempt to list common checks that PostgreSQL project Committers may want to adopt as part of a checklist of things to check before pushing. There are certain classic mistakes that even experienced committers have been known to make occasionally. In the real world, many mistakes happen when a step is skipped over during a routine process, perhaps caused by a seemingly insignificant last minute change. It's important to learn from these mistakes.

This checklist isn't intended as something that committers will adopt wholesale. Rather, it is intended as a starting point for creating your own semi-customized checklist. Since your final checklist is supposed to be used more or less mechanically, it shouldn't ever be too long, and should be organized into sections to make it easier to skip items where irrelevant. In short, if it's worth adopting something as a standard practice that you return to again and again, it's probably also worth writing that down, to formalize it. Use discretion when deciding what makes sense for you.

Basic checks

  • Double-check release build compiler warnings.
  • make check-world.
    • You may want to speed this up by using the following recipe:
make -j16 -s install;make -Otarget -j10 -s check-world  && echo "quick make-check world success" || echo "quick make-check world failure"
  • Consider the need for a catversion bump.
  • Don't assume that you haven't broken the doc build if you make even a trivial doc change.
    • Removing a GUC can break instances in the release notes where they're referenced.
    • Even grep can miss this, since references to the GUC will have dashes rather than underscores, plus possibly other variations.
  • Validate *printf calls for trailing newlines.
  • Spellcheck the patch.
  • Verify that long lines are not better broken into several shorter lines:
git diff origin/master | grep -E '^(\+|diff)' | sed 's/^+//' | expand -t4 | awk "length > 78 || /^diff/"
  • Run pgindent, pgperltidy, and reformat-dat-files on changed files; keep the changes minimal.
  • Run pgperlcritic on modified Perl files.
  • Update version numbers, if needed:
CATALOG_VERSION_NO, PG_CONTROL_VERSION, XLOG_PAGE_MAGIC, PGSTAT_FILE_FORMAT_ID
  • Update function/other OIDs, if needed;

Regression test checks

  • When adding core regression test files, make sure that they're added to both serial and parallel schedules.

(But release 14 and later have only the parallel schedule.)

  • Look for alternative output files for any regression test you're updating the output of.
    • Some tests have alternative output files to work around portability issues.
    • Most of the time it works to just apply the same patch to the other variants as the delta you're observing for the output file that's relevant to your own platform.
    • Occasionally you may have to just see what the buildfarm says.

Git checks

Basic

  • Do a dry run before really pushing by using --dry-run.
  • Look at "git status"; anything missing?
  • Author and committer timestamps should match.

This can be an issue if you're in the habit of rebasing, or apply a patch with "git am". Make sure that your setup displays both in "git log", by specifying "--pretty=fuller", or changing the git format config. The easiest way to make both timestamps match is to amend the commit like so: 

git commit --amend --reset-author

If you have "autosetuprebase = always" in your git config, then a last minute "git pull" could cause a rebase, which could cause author and committer timestamps to diverge a bit. In practice, small differences between author and committer timestamp are not considered to be a problem. 

Discussion: https://postgr.es/m/XXXXXXXXXXX
Back-patch depth?
What should the release notes say?
Credit any reviewer.
  • When making references to other commits, it's a good idea to use the first 9 chars of the commit SHA. Fewer than 9 means there will be no hyperlink in the HTTP interface. More than 9 is not required.
  • Note compatibility issues in commit message, so that they'll get picked up later, when release notes are written.
  • Check merge with master (not applicable to commits).
  • If you're using a dedicated ssh key with a passphrase, you may find it useful to deliberately disable it when you're done pushing:
$ ssh-add -d ~/.ssh/id_rsa_postgres

Backpatching and git

Commit messages for multiple branches should be identical when back-patching, in order to have tooling recognize the redundancy for purposes of compiling release notes, and other things of that nature.

  • Easiest way to get commit metadata consistent is to not worry about commit messages outside of the master branch at first. Commit message on backbranches could initially be something like "pending 9.6".
  • Perform the following procedure on each back branch when you're done, by checking out each individual branch in gitmaster local clone, and doing this for master branch commit which has good commit message:
git commit --amend --reset-author -C <commit>

You now have the same commit message on each branch. This means that the src/tools/git_changelog utility script will present the commits from each affected local branch together, as one logical change. (This script is used as a starting point when writing back branch release notes. Note that the concept of "one logical change" is not a standard git concept.)

  • Use git push origin : --dry-run to dry-run pushing all branches at once. Once satisfied, remove --dry-run to actually push. --dry-run is doubly important if you push each branch individually.

Maintaining ABI compatibility while backpatching

Avoid breaking ABI compatibility. It's unacceptable for extensions built against an earlier point release to break in a more recent point release.

  • You can only really change the signature of a function with local linkage, perhaps with a few rare exceptions.
  • You cannot modify any struct definition in src/include/*. If any new members must be added to a struct, put them at the end in backbranches. It's okay to have a different struct layout in master. Even then, extensions that allocate the struct can break via a dependency on its size.
  • Move new enum values to the end.

See this message for more considerations on ABI preservation.

GUC checks

  • When adding a new GUC, postgresql.conf.sample needs to be updated, too.
  • Is the GUC group the right one?

Advanced smoke tests

  • Valgrind memcheck + "make installcheck".
  • CLOBBER_CACHE_ALWAYS.
  • When doing anything that touches WAL-logging, consider creating a replica, and making sure that wal_consistency_checking=all passes on replica while master runs "make installcheck". WAL_DEBUG makes any bug that this throws up easier to isolate.
  • "#define COPY_PARSE_PLAN_TREES" and "#define WRITE_READ_PARSE_PLAN_TREES" can catch omissions or other mistakes when "src/backend/nodes/*" were changed.
  • check for unaligned access with things from c.h like -fsanitize=alignment
  • sqlsmith (for grammar changes, and ??)

Release freezes

Please be aware of upcoming release dates, and avoid committing into branches on which a release is scheduled, starting at noon UTC on the Saturday before the release date and extending until the release tags have been applied (which usually happens before midnight UTC on the Tuesday). This "quiet time" allows for full buildfarm testing before tarballs are made, and for correcting any packager trouble reports after the initial wrap. Security and release-note patches of course break this rule; otherwise, don't do it without consulting the release team.

Releases of supported back branches normally happen according to this schedule. Dates for beta, RC, and GA releases of a new branch are announced by the Release Management Team. Note that there is no freeze on the master branch unless a beta release is due to be made from it.

Retrieved from "https://wiki.postgresql.org/index.php?title=Committing_checklist&oldid=38139"