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"<ldoud...@redhat.com>
To:freeipa-devel@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

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to