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

[Lenka Doudova]

On 08/10/2016 05:48 PM, Martin Basti wrote:
>
>
>
> On 08.08.2016 10:30, Martin Basti wrote:
>>
>>
>>
>> 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
>
> As we figured out, pytest creates for each test new instance of class, 
> so 2) will not fork.
> Please fix only 1), sorry.
>
> Martin^2
Hi,
attached fixed patches for master and 4.3 branches.

Lenka

>
>> Martin^2
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160811/096e576b/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-placko-0003-3-ipa43-Test-URIs-in-certificate.patch
Type: text/x-patch
Size: 4768 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160811/096e576b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-placko-0003-3-Test-URIs-in-certificate.patch
Type: text/x-patch
Size: 4721 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160811/096e576b/attachment-0001.bin>