On Thu, Jan 23, 2025 at 12:30 PM Peter Smith <smithpb2...@gmail.com> wrote: > > Hi Shubham. > > The v12-0001 works OK, but your added TAP tests of this patch do not > conform to the new style of the fat-comma parameter passing since the > recent commit [1], so you'll need to fix them so they do... > > ====== > src/bin/pg_basebackup/t/040_pg_createsubscriber.pl > > 1. > +command_checks_all( > + [ > + 'pg_createsubscriber', '--verbose', > + '--verbose', '--recovery-timeout', > + "$PostgreSQL::Test::Utils::timeout_default", '--dry-run', > + '--pgdata', $node_s->data_dir, > + '--publisher-server', $node_p->connstr($db1), > + '--socketdir', $node_s->host, > + '--subscriber-port', $node_s->port, > + '--publication', 'pub1', > + '--publication', 'pub2', > + '--subscription', 'sub1', > + '--subscription', 'sub2', > + '--database', $db1, > + '--database', $db2 > + ], > > > 1a. > Fix the formatting. e.g. Look at master for example what new style > parameters look like and make yours look the same. > > ~ > > 1b. > Here I think we don't really any --verbose at all because AFAIK > command_checks_all is not producing any output we can look at anyhow. > > ~~~ > > 2. > +# And, same again to see the output... > command_ok( > [ > - 'pg_createsubscriber', > - '--verbose', > - '--dry-run', > - '--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', > - '--subscription' => 'sub1', > - '--subscription' => 'sub2', > - '--database' => $db1, > - '--database' => $db2, > + 'pg_createsubscriber', '--verbose', > + '--verbose', '--recovery-timeout', > + "$PostgreSQL::Test::Utils::timeout_default", '--dry-run', > + '--pgdata', $node_s->data_dir, > + '--publisher-server', $node_p->connstr($db1), > + '--socketdir', $node_s->host, > + '--subscriber-port', $node_s->port, > + '--publication', 'pub1', > + '--publication', 'pub2', > + '--subscription', 'sub1', > + '--subscription', 'sub2', > + '--database', $db1, > + '--database', $db2 > ], > > 2a. > +# And, same again to see the output... > > Maybe a better comment is more like > > # Execute the same command again but this time use 'command_ok' so > that the log files can be inspected. > > ~ > > 2b. > Fix the formatting. Look at master for example what new style > parameters look like and make yours look the same. > > ~ > > 2c. > Include some comment (like exists elsewhere in this file) to say > "--verbose is used twice to show more information.", otherwise readers > could be tempted to think the double --verbose is a mistake. > > ~~~ > > 3. > Rerun perltidy (the correct version) to ensure the formatting is > stable and good. > > ======
Fixed the given comments. The attached patch contains the suggested changes. Thanks and regards, Shubham Khanna.
v13-0001-Validate-max_slot_wal_keep_size-in-pg_createsubs.patch
Description: Binary data
