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

[Martin Basti]

On 11.08.2016 08:40, Lenka Doudova wrote:
>
>
>
> 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
>>>
>>>
>>
>
ACK

Pushed to master: 019f3611c299532cd89321767b0e0e4df0d0db27
Pushed to ipa-4-3: 2a207dd637748a4c05e54755b755986fbed16d55

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160811/440e7ef8/attachment.htm>