On Thu, 3 Apr 2025 at 09:07, Peter Smith <smithpb2...@gmail.com> wrote: > > Hi, > > I was away for the last few weeks when this feature got committed, so > I missed the chance to post my comments earlier. > > It seems the final SGML docs are mostly from this [1] suggestion, but > I think there are one or two more improvements that can be made to it: > > ====== > > 1. > It is not at all obvious from the current Options syntax that > -R/--remove must take a value. > A <replaceable>objtype</replaceable> should be included here to address that. > > Also, putting the "publications" as a <listitem> renders the HTML > better IMO, making it way easier to recognize that "publications" is > an object type, and also making future object types easier to add. > > ~ > > 2. markup > Use SGML <option> markup for --dry-run > Use SGML <application> markup for pg_dump > Use SGML <literal> markup for the specific object type value "publications" > > ~ > > 3. > Instead of "all tables" publications, we can call these FOR ALL TABLES > publications. That will be consistent with the Notes later on this > page. > > ~ > > 4. > "are individually logged and do show up in a --dry-run" > > I felt that "and do show up" is really just another way of saying "logged". > So, maybe reword this to say "are individually logged, including > during a --dry-run" > > ~~~ > > Please find attached a small patch that implements all these changes.
Thanks for the patch, the changes look good to me. Regards, Vignesh
