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

[Martin Basti]

On 30.06.2016 15:16, Florence Blanc-Renaud wrote:
> 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.
>
Pushed to master: 3fab1b63502c3206d792b7aeaa12d486612f0137