Bruce Momjian <br...@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


Reply via email to