From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Tristen Raab <tristen(dot)raab(at)highgo(dot)ca> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Documentation: warn about two_phase when altering a subscription |
Date: | 2024-02-26 09:14:07 |
Message-ID: | ZdxWX2hmEi82yOew@ip-10-97-1-34.eu-west-3.compute.internal |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On Fri, Feb 23, 2024 at 11:50:17PM +0000, Tristen Raab wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world: not tested
> Implements feature: tested, passed
> Spec compliant: not tested
> Documentation: tested, passed
>
> Hello,
>
> I've reviewed your patch,
Thanks!
> it applies correctly and the documentation builds without any errors. As for the content of the patch itself, I think so far it's good but would make two modifications. I like how the patch was worded originally when referring to the subscription, stating these parameters were 'in' the subscription rather than 'by' it. So I'd propose changing
>
> > parameters specified by the subscription. When creating the slot, ensure
>
> to
>
> > parameters specified in the subscription. When creating the slot, ensure
As non native English speaker I don't have a strong opinion on that (though I
also preferred how it was worded in V1).
>
> and secondly the section "ensure the slot properties failover and two_phase match their counterpart parameters of the subscription" sounds a bit clunky. So I'd also propose changing:
>
> > the slot properties <literal>failover</literal> and <literal>two_phase</literal>
> > match their counterpart parameters of the subscription.
>
> to
>
> > the slot properties <literal>failover</literal> and <literal>two_phase</literal>
> > match their counterpart parameters in the subscription.
Same here, I don't have a strong opinion on that.
As the patch as it is now looks good to Amit (see [1]), I prefer to let him
decide which wording he pefers to push.
> I feel this makes the description flow a bit better when reading. But other than that I think it's quite clear.
Thanks!
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Andrei Lepikhov | 2024-02-26 09:30:50 | Re: "type with xxxx does not exist" when doing ExecMemoize() |
Previous Message | Andrey M. Borodin | 2024-02-26 09:10:49 | Re: Injection points: some tools to wait and wake |