Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,  Tom Lane <tgl@sss.pgh.pa.us>,
  PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Peter Eisentraut
 <peter@eisentraut.org>, Euler Taveira <euler.taveira@enterprisedb.com>
Subject: Re: pg_createsubscriber TAP test wrapping makes command options
 hard to read.
References: 
 <CAHut+PvfaUZQR3rEnj-Dcg_6713Wu4yRdd-yZkkC9NVsARht8A@mail.gmail.com>
	<556792.1733967676@sss.pgh.pa.us>
	<CAHut+Ps6Q+i_3xS6H7ybw3-hx9+2QOuneTJwHArxqxFnKX5+4g@mail.gmail.com>
	<564915.1733971586@sss.pgh.pa.us>
	<CAHut+PvFGytgJT+K_e2x+uWKuVYCBR3HNaNtPKYEkO3YxAwZvw@mail.gmail.com>
	<Z1peKSb8roiDNgRj@paquier.xyz>
	<CAHut+PvYsVhF5k-mYHVEFdpijdgkYRtoeWVxdspxio0ePRpkug@mail.gmail.com>
	<87pllx5cqn.fsf@wibble.ilmari.org> <87msh159c3.fsf@wibble.ilmari.org>
	<CAHut+Ptmusd=7SdZ-n0=fu-Y8ut=UaK9nU4Cqk01wxOmio9nkA@mail.gmail.com>
Date: Mon, 20 Jan 2025 20:42:45 +0000
In-Reply-To: 
 <CAHut+Ptmusd=7SdZ-n0=fu-Y8ut=UaK9nU4Cqk01wxOmio9nkA@mail.gmail.com>
	(Peter Smith's message of "Tue, 14 Jan 2025 18:03:23 +1100")
Message-ID: <87o70143p6.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Archived-At: 
 <https://www.postgresql.org/message-id/87o70143p6.fsf%40wibble.ilmari.org>
Precedence: bulk

Hi Peter,

Peter Smith <smithpb2250@gmail.com> writes:

> Hi,
>
> In your v4 patch, there is a fragment (below) that replaces a  double
> '--verbose' switch with just a single '--verbose'.
>
> As I have only recently learned, the '--verbose'' switch has a
> cumulative effect [1], so the original double '--verbose' was probably
> deliberate so it should be kept that way.

I was going to say that if it were deliberate, I would have expected the
two `--verbose` swithces to be together, but then I noticed that the
--recovery-timeout switch was added later (commit 04c8634c0c4d), rather
haphazardly between them.  Still, I would have expected a comment as to
why this command was being invoked extra-verbosely, but I'll restore it
in the next version.

I've Cc-ed both Peter (the committer) and Euler (the author) in case
they have any insight.

> ~~
>
>  # Run pg_createsubscriber on node S
>  command_ok(
>   [
> - 'pg_createsubscriber', '--verbose',
> - '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
> - '--verbose', '--pgdata',
> - $node_s->data_dir, '--publisher-server',
> - $node_p->connstr($db1), '--socketdir',
> - $node_s->host, '--subscriber-port',
> - $node_s->port, '--publication',
> - 'pub1', '--publication',
> - 'Pub2', '--replication-slot',
> - 'replslot1', '--replication-slot',
> - 'replslot2', '--database',
> - $db1, '--database',
> - $db2
> + 'pg_createsubscriber',
> + '--verbose',
> + '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default,
> + '--pgdata' => $node_s->data_dir,
> + '--publisher-server' => $node_p->connstr($db1),
> + '--socketdir' => $node_s->host,
> + '--subscriber-port' => $node_s->port,
> + '--publication' => 'pub1',
> + '--publication' => 'pub2',
> + '--replication-slot' => 'replslot1',
> + '--replication-slot' => 'replslot2',
> + '--database' => $db1,
> + '--database' => $db2,
>   ],
>   'run pg_createsubscriber on node S');
>
>
> ======
> [1] https://www.postgresql.org/docs/devel/app-pgcreatesubscriber.html
>
> Kind Regards,
> Peter Smith.
> Fujitsu Australia