Michael Paquier <mich...@paquier.xyz> writes: > 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.
Thanks, I'm glad you agree. > 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. I did it by grepping for the various functions that call programs, such as command_((fails_)?like|ok) etc, and then reading the docs for each command to find the corresponding long options. It got almost meditative after a while, especially once I got some Emacs keyboard macros dialled in. The ones in src/bin/pg_resetwal/t/001_basic.pl I deliberatly left short because the error messages refer to the short spelling even if you used the long one. > 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. Yeah, good point. > The option should be named --skip-checksums, causing a test failure in > the CI. All the tests passed for me on both Debian and macOS, I guess their getopts accept unambiguous abbreviations of options. > -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. Yeah. > 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. Thanks again for reviewing this monster patch. - ilmari
