Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 21 Jan 2025 16:40:18 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Dagfinn Ilmari =?iso-8859-1?Q?Manns=E5ker?= <ilmari@ilmari.org>,
	Andrew Dunstan <andrew@dunslane.net>, Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_createsubscriber TAP test wrapping makes command options hard
 to read.
Message-ID: <Z49PYgXnbyU6IiEe@paquier.xyz>
References: <87pllx5cqn.fsf@wibble.ilmari.org>
 <87msh159c3.fsf@wibble.ilmari.org>
 <0f5256ec-d63e-4e0f-9b71-750a7a4e5c01@dunslane.net>
 <651128.1734017715@sss.pgh.pa.us>
 <87jzc46d8u.fsf@wibble.ilmari.org>
 <5f41d6c6-1cbb-494d-a5c4-3c449fc4e9ad@dunslane.net>
 <105f432a-6b0b-42d4-9061-65d263279be7@app.fastmail.com>
 <CAHut+PuD0aQfjAN0q_7FpdNdSEdrhONpTsb0tQi5ZEtZHj6bKg@mail.gmail.com>
 <87ldv542fk.fsf@wibble.ilmari.org>
 <CAHut+PvKrQXY7kM8ct=HC9xYPC6yhdJWnvCgijtgxmO-mus8uQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="8EQK0bdRxhKIzAGY"
Content-Disposition: inline
In-Reply-To: <CAHut+PvKrQXY7kM8ct=HC9xYPC6yhdJWnvCgijtgxmO-mus8uQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Z49PYgXnbyU6IiEe%40paquier.xyz>
Precedence: bulk


--8EQK0bdRxhKIzAGY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 21, 2025 at 10:31:01AM +1100, Peter Smith wrote:
> I applied the v5* patches and ran make check-world. All passed OK.
>=20
> Your fat comma changes have greatly improved readability, particularly
> in 040_createsubscriber (the file that caused me to start this
> thread). Thanks!

It is not an improvement in only 040_createsubscriber, all the places
Dagfinn is touching here feel much better with this new syntax.  You
have spent a lot of time on the change from short to long names and on
the new convention grammar for the option/value duos, clearly.

It took me some time to read through the patch to catch
inconsistencies.

-   [ 'pg_verifybackup', '-s', $backup_path ],
+   [ 'pg_verifybackup', '--skip-checksum', $backup_path ],
    qr/backup successfully verified/,
    '-s skips checksumming');

This one in pg_verifybackup should perhaps switch to the long option
for the test description, like the rest.  The option should be named
--skip-checksums, causing a test failure in the CI.

-my @pg_basebackup_cmd =3D (@pg_basebackup_defs, '-U', 'backupuser', '-Xfet=
ch');
+my @pg_basebackup_cmd =3D (
+	@pg_basebackup_defs,
+	'--user' =3D> 'backupuser',

There was a second series of failures in the test
basebackup_to_shell/t/001_basic.pl because of this part.  The option
name should be "--username" for pg_basebackup.

With these two fixes, the CI is happy, which should hopefully be
enough for the buildfarm.  But we'll see..

I'd like to apply what you have done here (planning to do a second
lookup, FWIW).  If anybody has objections, feel free to post that
here.
--
Michael

--8EQK0bdRxhKIzAGY
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmePT2IACgkQnvQgOdby
QH1MTA//dFe7WYfFJRKc/IS5KNG5zxWtC/6+4mq6aZ+FXVaWYRWpvA/rBCloYWAm
Tn4kly7H8f/MI3kIODdosMOT9XVInKg92AaXqCqoJxRhcuFwSIjD86bLwYJl+SUN
xJPblUl/90UWNOy7WmX8tTpKUQTpmFI7q7nzxjiwNvEbmJELPUyGzDsy0ZO40vnD
PkDTdkpzqXretZNSkK8usoaFMhOjFTEsxQDmYnof1RmpZcrzuUk+5j+dZ/yttRiA
2hPCBe2+oai8xXHEuPs5n1f/rSooiVDXOuChRJaHq9rPKQQQlz5Albq3iyg0gk7G
5DDXDKNbmv/IgpsWVREWv0VkVEYfYK7PP8CthYJi88K1lvDzRmOa9N9DDQ+QGPoz
lrCFneQsrLofiXebSTa3u+wvm6+FO3CvzK0JsFQyiK0L6x/tAsZ2r7nddDU9sMik
5khs+ooJ/aTOHuWgMly/SfJdOKY6gPx+J3hgygst8ZCziRIM+RGKn/Bs5BagMbwX
EGR0T1wfEkOBq5+SZs6PNJ0xwTPV/3sdm2iwdUs67PrpiWgb3Lfymo2D1whDB94V
1pis2qT/mz2YKhPwnkDjiwlaix+oZat6Om55rdWw50okCJwVWVArOk249t40LAVD
tRf6fEyvbnPhGkceK08CQs7R2gd+phWJX9M8zeQ6iaUzjrhA/Fo=
=cMw6
-----END PGP SIGNATURE-----

--8EQK0bdRxhKIzAGY--