On Sat, Mar 15, 2025 at 03:26:10PM +0900, Michael Paquier wrote: > Thanks for digging into all that. Agreed that it would be nice to > apply a more consistent style for this release. I'll look more into > what you have here.
The changes in pg_dump's 010_dump_connstr.pl read the same. extension_schema in test_pg_dump adds silently a --no-sync. > Hmm. I can partially get behind this one, seeing things like that in > what you have sent: > > @@ -24,8 +24,8 @@ my $node = PostgreSQL::Test::Cluster->new('primary'); > # Make sure pg_hba.conf is set up to allow connections from backupuser. > # This is only needed on Windows machines that don't use UNIX sockets. > $node->init( > - 'allows_streaming' => 1, > - 'auth_extra' => [ '--create-role' => 'backupuser' ]); > + allows_streaming => 1, > + auth_extra => [ '--create-role' => 'backupuser' ]); > > This option handling style is inconsistent in the tree. Most of the > time these keywords are not quoted when it comes to our internal test > modules.. So even for the option handling, what we have here is a mixed bad of inconsistencies and rather consistent-ish behaviors. Some notes: src/bin/pg_basebackup/t/010_pg_basebackup.pl quotes tablespace_map. src/bin/pg_basebackup/t/011_in_place_tablespace.pl quotes allows_streaming. slot_type and restart_lsn are quoted everywhere now. src/bin/pg_combinebackup/t/008_promote.pl quotes has_streaming. ENV is a mixed bag of various things, single, double or no quotes. pg_verifybackup/t/003_corruption.pl, 008, 009, 010 are a set of local changes. 001_uri.pl was inconsistent, still local. The parts about auth_extra are inconsistent (like in 002_connection_limits.pl). Inconsistency in BackgroundPsql.pm, but it's always quoted now. 021_row_visibility.pl and 032_relfilenode_reuse.pl are local. Hm, not planning to bother here. Okay for 003_sslinfo.pl and 002_scram.pl, that mix both styles, unquoting looks fine. Not sure about the build scripts as a whole. One point that applies for sure is that some existing options use an inconsistent style across the tree in the TAP tests, mainly for init() and init_from_backup(). I've extracted these from 0002, and applied the subset. For the rest, if there are more opinions, feel free.. -- Michael
signature.asc
Description: PGP signature