On Thu, 3 Apr 2025 at 09:07, Peter Smith <smithpb2250@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
