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 documentation builds without any errors. As for the content of
thepatch itself, I think so far it's good but would make two modifications. I like how the patch was worded originally
whenreferring to the subscription, stating these parameters were 'in' the subscription rather than 'by' it. So I'd
proposechanging
> parameters specified by the subscription. When creating the slot, ensure
to
> parameters specified in the subscription. When creating the slot, ensure
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.
I feel this makes the description flow a bit better when reading. But other than that I think it's quite clear.
kind regards,
-----------------------
Tristen Raab
Highgo Software Canada
www.highgo.ca