Re: Small documentation improvement for ALTER SUBSCRIPTION

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Small documentation improvement for ALTER SUBSCRIPTION
Date: 2021-08-10 02:17:07
Message-ID: CAHut+PuFx8ZDGk24qvJeKjqiBoZ0hPwNHp+7k4UoAaQB8uzH0A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 10, 2021 at 11:01 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Mon, Aug 9, 2021 at 1:01 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > On Mon, Aug 9, 2021 at 12:46 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Sun, Aug 8, 2021 at 10:21 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > > >
> > > > On Sat, Aug 7, 2021 at 4:33 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > >
> > > > > On Thu, Jul 8, 2021 at 6:31 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > When reading the doc of ALTER SUBSCRIPTION I realized that 'refresh
> > > > > > options' in the following paragraph is not tagged:
> > > > > >
> > > > > > ---
> > > > > > Additionally, refresh options as described under REFRESH PUBLICATION
> > > > > > may be specified, except in the case of DROP PUBLICATION.
> > > > > > ---
> > > > > >
> > > > > > When I read it for the first time, I got confused because we actually
> > > > > > have the 'refresh' option and this description in the paragraph of the
> > > > > > 'refresh' option. I think we can improve it by changing to
> > > > > > '<replaceable>refresh_option</replaceable>'. Thoughts?
> > > > > >
> > > > >
> > > > > I see that one can get confused but how about changing it to
> > > > > "Additionally, refresh options as described under <literal>REFRESH
> > > > > PUBLICATION</literal> (<replaceable>refresh_option</replaceable>) may
> > > > > be specified,.."? I think keeping "refresh options" in the text would
> > > > > be good because there could be multiple such options.
> > > > >
> > > >
> > > > I feel like it would be better to reword it in some way that avoids
> > > > using parentheses because they look like part of the syntax instead of
> > > > just part of the sentence.
> > > >
> > >
> > > Fair enough, feel free to propose if you find something better or if
> > > you think the current text in the docs is good.
> > >
> >
>
> Thank you for the comments!
>
> > IMO just the same as your suggestion but without the parens would be good. e.g.
> >
> > "Additionally, refresh options as described under <literal>REFRESH
> > PUBLICATION</literal> <replaceable>refresh_option</replaceable> may be
> > specified,.."
>
> But "REFRESH PUBLICATION refresh_option" seems wrong in terms of SQL
> syntax, not?
>

Because the sentence says "... as described under ..." I thought it
was clear enough it was referring to the documentation below and not
the SQL syntax.

> Given there could be multiple options how about using
> "<replaceable>refresh_options</replaceable>"? That is, the sentence
> will be:
>
> Additionally, <replaceable>refresh_options</replaceable> as described
> under <literal>REFRESH PUBLICATION</literal> may be specified,
> except in the case of <literal>DROP PUBLICATION</literal>.
>

+1 LGTM

------
Kind Regards,
Peter Smith.
Fujitsu Australia.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-08-10 02:20:33 Re: Another regexp performance improvement: skip useless paren-captures
Previous Message Mark Dilger 2021-08-10 01:38:33 Re: Another regexp performance improvement: skip useless paren-captures