Re: pg_upgrade: Make testing different transfer modes easier

At Wed, 14 Dec 2022 10:40:45 +0100, Daniel Gustafsson <daniel@yesql.se> wrote in 
> > On 14 Dec 2022, at 08:04, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
> > 
> > On 07.12.22 17:33, Peter Eisentraut wrote:
> >> I think if we want to make this configurable on the fly, and environment variable would be much easier, like
> >>     my $mode = $ENV{PG_TEST_PG_UPGRADE_MODE} || '--copy';
> > 
> > Here is an updated patch set that incorporates this idea.

We have already PG_TEST_EXTRA. Shouldn't we use it here as well?

> I would prefer a small note about it in src/bin/pg_upgrade/TESTING to document
> it outside of the code, but otherwise LGTM.
> 
> +        $mode,
>          '--check'
>      ],
> 
> ...
> 
> -        '-p',         $oldnode->port,     '-P', $newnode->port
> +        '-p',         $oldnode->port,     '-P', $newnode->port,
> +        $mode,
>      ],
> 
> Minor nitpick, but while in there should we take the opportunity to add a
> trailing comma on the other two array declarations which now ends with --check?
> It's good Perl practice and will make the code consistent.

Otherwise look god to me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center