> On 30 Mar 2021, at 11:53, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Mar 30, 2021 at 03:50:28PM +0900, Michael Paquier wrote:
>> The test_*() ones are just wrappers for psql able to use a customized
>> connection string. It seems to me that it would make sense to move
>> those two into PostgresNode::psql itself and extend it to be able to
>> handle custom connection strings?
>
> Looking at this part, I think that this is a win in terms of future
> changes for SSLServer.pm as it would become a facility only in charge
> of managing the backend's SSL configuration. This has also the
> advantage to make the error handling with psql more consistent with
> the other tests.
>
> So, attached is a patch to do this simplification. The bulk of the
> changes is within the tests themselves to adapt to the merge of
> $common_connstr and $connstr for the new routines of PostgresNode.pm,
> and I have done things this way to ease the patch lookup. Thoughts?
LGTM with the findings that Alvaro reported.
+$node->connect_ok($common_connstr . " " . "user=ssltestuser",
This double concatenation could be a single concat, or just use scalar value
interpolation in the string to make it even more readable. As it isn't using
the same line broken pattern of the others the concat looks a bit weird as a
result.
Thanks for picking it up, as I have very limited time for hacking right now.
--
Daniel Gustafsson https://vmware.com/
