Re: Documentation: warn about two_phase when altering a subscription

2024-03-10 Thread Amit Kapila
On Sat, Feb 24, 2024 at 5:21 AM Tristen Raab wrote: > > I've reviewed your patch, 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 >

Re: Documentation: warn about two_phase when altering a subscription

2024-03-02 Thread Amit Kapila
On Sat, Mar 2, 2024 at 11:44 PM Andrey M. Borodin wrote: > > > On 26 Feb 2024, at 14:14, Bertrand Drouvot > > wrote: > > > > 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. > > Added Amit to cc, just to be sure. Thanks! >

Re: Documentation: warn about two_phase when altering a subscription

2024-03-02 Thread Andrey M. Borodin
> On 26 Feb 2024, at 14:14, Bertrand Drouvot > wrote: > > 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. Added Amit to cc, just to be sure. Thanks! Best regards, Andrey Borodin, learning how to be CFM.

Re: Documentation: warn about two_phase when altering a subscription

2024-02-26 Thread Bertrand Drouvot
Hi, On Fri, Feb 23, 2024 at 11:50:17PM +, 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,

Re: Documentation: warn about two_phase when altering a subscription

2024-02-23 Thread Tristen Raab
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, it applies correctly and the

Re: Documentation: warn about two_phase when altering a subscription

2024-02-02 Thread Amit Kapila
On Wed, Jan 31, 2024 at 11:25 AM Bertrand Drouvot wrote: > > They make sense to me so applied both in v2 attached. > This patch looks good to me but I think it is better to commit this after we push some more work on failover slots, especially the core sync slot functionality because we are

Re: Documentation: warn about two_phase when altering a subscription

2024-01-31 Thread Peter Smith
On Wed, Jan 31, 2024 at 4:55 PM Bertrand Drouvot wrote: ... > As a non native English speaker somehow I have to rely on you for those > suggestions ;-) > > They make sense to me so applied both in v2 attached. > Patch v2 looks OK to me, but probably there is still room for improvement. Let's see

Re: Documentation: warn about two_phase when altering a subscription

2024-01-30 Thread Bertrand Drouvot
Hi, On Wed, Jan 31, 2024 at 01:47:16PM +1100, Peter Smith wrote: > Hi, thanks for the patch. Here are some review comments for v1 Thanks for the review! > > == > > (below is not showing the links and other sgml rendering -- all those LGTM) > > BEFORE > When altering the slot_name, the

Re: Documentation: warn about two_phase when altering a subscription

2024-01-30 Thread Peter Smith
Hi, thanks for the patch. Here are some review comments for v1 == (below is not showing the links and other sgml rendering -- all those LGTM) BEFORE When altering the slot_name, the failover and two_phase properties values of the named slot may differ from their counterparts failover and

Documentation: warn about two_phase when altering a subscription

2024-01-30 Thread Bertrand Drouvot
Hi hackers, 776621a5e4 added a "warning" in the documentation to alter a subscription (to ensure the slot's failover property matches the subscription's one). The same remark could be done for the two_phase option. This patch is an attempt to do so. Looking forward to your feedback, Regards,