Bruce Momjian <bruce@momjian.us> writes:
> On Sun, May 21, 2023 at 08:22:05AM -0700, Nathan Bossart wrote:
>> I've attached a draft patch for this. I also changed --admin to
>> --with-admin.
> If we want to go forward with this, the big question is whether we want
> to get this in before beta1. FYI, the release notes don't mention the
> option names.
+1 for doing it before beta1.
A few comments on the patch:
>> Indicates an existing role that will be automatically added as a member of the new
"Specifies" would be clearer than "indicates" (not your fault, but
let's avoid the passive construction while we are here). Likewise
nearby.
>> + {"member-of", required_argument, NULL, 6},
Why didn't you just translate this as 'g' instead of inventing
a new switch case?
>> - printf(_(" -a, --admin=ROLE this role will be a member of new role with admin\n"
>> + printf(_(" -a, --with-admin=ROLE this role will be a member of new role with admin\n"
I think clearer would be
>> + printf(_(" -a, --with-admin=ROLE ROLE will be a member of new role with admin\n"
Likewise
>> + printf(_(" -g, --member-of=ROLE new role will be a member of ROLE\n"));
(I assume that's what this should say, it's backwards ATM)
and
>> + printf(_(" -m, --with-member=ROLE ROLE will be a member of new role\n"));
regards, tom lane