On Mon, Jul 10, 2023 at 3:44 AM Heikki Linnakangas <hlinn...@iki.fi> wrote: > I'm late to the party, but regarding commit c03c2eae0a, which added the > guidelines for writing formatting desc functions: > > You moved the comment from rmgrdesc_utils.c into rmgrdesc_utils.h, but I > don't think that was a good idea. Our usual convention is to have the > function comment in the .c file, not at the declaration in the header > file. When I want to know what a function does, I jump to the .c file, > and might miss the comment in the header entirely. > > Let's add a src/backend/access/rmgrdesc/README file. We don't currently > have any explanation anywhere why the rmgr desc functions are in a > separate directory. The README would be a good place to explain that, > and to have the formatting guidelines. See attached.
diff --git a/src/backend/access/rmgrdesc/README b/src/backend/access/rmgrdesc/README new file mode 100644 index 0000000000..abe84b9f11 --- /dev/null +++ b/src/backend/access/rmgrdesc/README @@ -0,0 +1,60 @@ +src/backend/access/rmgrdesc/README + +WAL resource manager description functions +========================================== + +For debugging purposes, there is a "description function", or rmgrdesc +function, for each WAL resource manager. The rmgrdesc function parses the WAL +record and prints the contents of the WAL record in a somewhat human-readable +format. + +The rmgrdesc functions for all resource managers are gathered in this +directory, because they are also used in the stand-alone pg_waldump program. "standalone" seems the more common spelling of this adjective in the codebase today. +They could potentially be used by out-of-tree debugging tools too, although +the the functions or the output format should not be considered a stable API. You have an extra "the". I might phrase the last bit as "neither the description functions nor the output format should be considered part of a stable API" +Guidelines for rmgrdesc output format +===================================== I noticed you used === for both headings and wondered if it was intentional. Other READMEs I looked at in src/backend/access tend to have a single heading underlined with ==== and then subsequent headings are underlined with -----. I could see an argument either way here, but I just thought I would bring it up in case it was not a conscious choice. Otherwise, LGTM. - Melanie
