Re: Changing the state of data checksums in a running cluster

On 3/11/25 14:07, Dagfinn Ilmari Mannsåker wrote:
> As the resident perl style pedant, I'd just like to complain about the
> below:
> 
> Tomas Vondra <tomas@vondra.me> writes:
> 
>> diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
>> index 666bd2a2d4c..1c66360c16c 100644
>> --- a/src/test/perl/PostgreSQL/Test/Cluster.pm
>> +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
>> @@ -3761,7 +3761,8 @@ sub checksum_enable_offline
>>      my ($self) = @_;
>>  
>>      print "### Enabling checksums in \"$self->data_dir\"\n";
>> -    PostgreSQL::Test::Utils::system_or_bail('pg_checksums', '-D', $self->data_dir, '-e');
>> +    PostgreSQL::Test::Utils::system_or_bail('pg_checksums', '-D',
>> +        $self->data_dir, '-e');
>>      return;
>>  }
> 
> 
> This breaking between the command line options and its arguments is why
> we're switching to using fat commas. We're also using long options for
> improved self-documentation, so this should be written as:
> 
>     PostgreSQL::Test::Utils::system_or_bail('pg_checksums',
>         '--pgdata' => $self->data_dir,
>         '--enable');
> 
> And likewise below in the disable method.
> 

I don't know what fat comma is, but that's simply what perltidy did. I
don't mind formatting it differently, if there's a better way.


thanks

-- 
Tomas Vondra