Am 10.03.22 um 14:43 schrieb Michael Banck:
> Heya,
>
> some minor comments, I didn't look at the added test and I did not test
> the patch at all, but (as part of the Debian/Ubuntu packaging team) I
> think this patch is really important:
Much appreciated!
[...]
> I think we usually just say "data directory"; pgdata was previously only
> used as part of the option name, not like this in the text.
Fixed.
[...]
> target-pgdata is the name of the option, but maybe it is useful to spell
> out "target data directory" here, even if that means we spill over to
> two lines (which we might have to do anyway, that new line is pretty
> long).
Fixed.
>
>> @@ -121,6 +123,7 @@ main(int argc, char **argv)
>> {"no-sync", no_argument, NULL, 'N'},
>> {"progress", no_argument, NULL, 'P'},
>> {"debug", no_argument, NULL, 3},
>> + {"config-file", required_argument, NULL, 5},
>
> Not sure what our policy is, but the way the numbered options are 1, 2,
> 4, 3, 5 after this is weird; this was already off before this patch
> though.
That one has been triggering my inner Monk too ;-)
Fixed!
[...]
> You removed an empty line here. Maybe rather prepend this with a comment
> on what's going on, and have en empty line before and afterwards.
> Kinde same here.
It does smell a bit like "stating the obvious", but alas! Both fixed.
Cheers,
--
Gunnar "Nick" Bluth
Eimermacherweg 106
D-48159 Münster
Mobil +49 172 8853339
Email: gunnar.bluth@pro-open.de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato
