[Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate

[Martin Basti]

On 02.08.2016 14:50, Lenka Doudova wrote:
>
>
>
> On 07/29/2016 11:43 AM, Lenka Doudova wrote:
>>
>>
>>
>> On 07/29/2016 11:41 AM, Lenka Doudova wrote:
>>>
>>> On 07/28/2016 01:35 PM, Peter Lacko wrote:
>>>> Hops, fixed.
>>>>
>>>> Peter
>>>>
>>>>
>>>> ----- Original Message -----
>>>> From: "Lenka Doudova"<ldoudova at redhat.com>
>>>> To:freeipa-devel at redhat.com
>>>> Sent: Thursday, July 28, 2016 1:32:25 PM
>>>> Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in	certificate
>>>>
>>>> Hi,
>>>>
>>>> I cannot find any attached patch :)
>>>>
>>>> Lenka
>>>>
>>>>
>>>> On 07/28/2016 01:30 PM, Peter Lacko wrote:
>>>>> Attached you can find a patch adding test for URIs in generated certificate
>>>>> ipatests/test_xmlrpc/test_cert_plugin.py
>>>>> Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Peter
>>>>>
>>>>
>>>>
>>> Hi,
>>>
>>> NACK. Code looks fine and works well on master branch, but patch 
>>> does not apply on 4-3 and 4-2 branches.
>>> Peter left the company but claimed he can fix the patch if 
>>> necessary, I'll communicate it with him or fix it myself.
>>>
>>> Lenka
>>>
>>>
>> Oh, and forgot this one - PEP8 error:
>> ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long 
>> (105 > 79 characters)
>>
>> Lenka
>>
>>
> Hi,
>
> since Peter has quit already, I took it upon myself to do minor fix 
> and rebase to the patch.
> 1) i removed pylint disable comments from the patch, as they were 
> unnecessary (this also solved PEP8 error)
> 2) I rebased the patch to be applicable for ipa-4-3 branch.
> Original functionality of the patch remains unchanged.
>
> Both fixed patches attached.
>
> Lenka
>
>

Hello,

1)
This is not needed

+        global sn
+
+        result = api.Command.cert_show(sn, out=unicode(self.certfile))

you need the global statement only for write access. But sn is not assigned in this code block.

2)
I prefer to use instance attributes (self.sn) instead of global variables


Martin^2

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160808/340dc95c/attachment.htm>