[Freeipa-devel] [PATCH] 0082 cert-request: better error msg when 'add' not supported

[Fraser Tweedale]

On Thu, Jun 30, 2016 at 11:38:35AM +0200, Florence Blanc-Renaud wrote:
> On 06/30/2016 06:29 AM, Fraser Tweedale wrote:
> > On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud wrote:
> > > On 06/29/2016 07:25 AM, Fraser Tweedale wrote:
> > > > The attached patch fixes
> > > > https://fedorahosted.org/freeipa/ticket/5991.
> > > > 
> > > > Thanks,
> > > > Fraser
> > > > 
> > > > 
> > > > 
> > > Hi Fraser,
> > > 
> > > A few cosmetic comments:
> > > 
> > > PEP8 issues:
> > > ./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1
> > > ./ipaserver/plugins/cert.py:394:80: E501 line too long (98 > 79 characters)
> > > ./ipaserver/plugins/cert.py:496:80: E501 line too long (81 > 79 characters)
> > > 
> > > and there is a typo in ipaserver/plugins/cert.py
> > > +            doc=_("automatically add the principal if it doesn't exist
> > > (service princpals only)"),
> > > 
> > > should be "princ*i*pals only"
> > > 
> > > Otherwise LGTM,
> > > Flo
> > > 
> > Thanks for review, Flo.  Updated patch attached.
> > 
> > Cheers,
> > Fraser
> > 
> Hi Fraser,
> 
> thanks for updated patch. There is still a pep8 error:
> ./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1
> 
Whups!

> I am also wondering if the message is clear enough. When running the CLI
> it's ok because the user typed --add, but the GUI displays "'add' is not
> supported for user principals"
> and I feel
> "'add' option is not supported for user principals"
> would be more user-friendly. What do you think?
> 
Yes, I concur that mentioning "option" is an improvement.  Will cut
a new patch shortly

Thanks!
Fraser