Re: alter subscription drop publication fixes

On Sat, May 15, 2021 at 2:58 PM Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Fri, May 14, 2021 at 11:02 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Fri, May 14, 2021 at 8:56 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >
> > > Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> writes:
> > > > On Fri, May 14, 2021 at 7:58 PM vignesh C <vignesh21(at)gmail(dot)com>
wrote:
> > > >> I have changed it to:
> > > >> <literal>ADD</literal> adds additional publications,
> > > >> - <literal>DROP</literal> removes publications from the list
of
> > > >> + <literal>DROP</literal> removes publications to/from the
list of
> > >
> > > > How about "Publications are added to or dropped from the existing
list
> > > > of publications by <literal>ADD</literal> or
<literal>DROP</literal>
> > > > respectively." ?
> > >
> > > We generally prefer to use the active voice, so I don't think
> > > restructuring the sentence that way is an improvement. The quoted
> > > bit would be better left alone entirely. Or maybe split it into
> > > two sentences, but keep the active voice.
> >
> > I felt changing it to the below was better:
> > SET replaces the entire list of publications with a new list, ADD adds
> > additional publications to the list of publications and DROP removes
> > the publications from the list of publications.
> >
> > Attached patch has the change for the same.
> > Thoughts?
>
> Thanks Vignesh, the patch looks good to me and it works as expected
> i.e. doesn't show up the copy_data option in the tab complete for the
> alter subscription drop publication command. While on this, I observed
> that the new function merge_publications and the error message crossed
> the 80char limit, I adjusted that and added a commit message. Please
> have a look, if that is okay, add an entry to the commit fest and pass
> it on to the committer as I have no further comments.

Thanks Bharath, that looks good. I have added a commitfest entry at [1] and
marked it to Ready For Committer.
[1] - https://commitfest.postgresql.org/33/3115/

Regards,
Vignesh