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

[Florence Blanc-Renaud]

On 06/30/2016 01:30 PM, Fraser Tweedale wrote:
> On Thu, Jun 30, 2016 at 07:49:04PM +1000, Fraser Tweedale wrote:
>> 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
>>
> Updated patch attached.  Third time's the charm? ;)
>
> Cheers,
> Fraser
>
Hi Fraser,

thanks for the updated patch. Yes, 3rd time's the charm :)
Ack,
Flo.