public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Peter Smith <[email protected]>
Cc: Dagfinn Ilmari Mannsåker <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_createsubscriber TAP test wrapping makes command options hard to read.
Date: Tue, 21 Jan 2025 16:40:18 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHut+PvKrQXY7kM8ct=HC9xYPC6yhdJWnvCgijtgxmO-mus8uQ@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAHut+PuD0aQfjAN0q_7FpdNdSEdrhONpTsb0tQi5ZEtZHj6bKg@mail.gmail.com>
	<[email protected]>
	<CAHut+PvKrQXY7kM8ct=HC9xYPC6yhdJWnvCgijtgxmO-mus8uQ@mail.gmail.com>

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.
> 
> 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 = (@pg_basebackup_defs, '-U', 'backupuser', '-Xfetch');
+my @pg_basebackup_cmd = (
+	@pg_basebackup_defs,
+	'--user' => '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


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_createsubscriber TAP test wrapping makes command options hard to read.
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox