<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 05/31/2013 12:25 PM, Tomas Babej
wrote:<br>
</div>
<blockquote cite="mid:51A87A96.6080404@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 05/30/2013 02:17 PM, Ana
Krivokapic wrote:<br>
</div>
<blockquote cite="mid:51A7436F.7060302@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 05/13/2013 05:42 PM, Tomas Babej
wrote:<br>
</div>
<blockquote cite="mid:519109CC.5090300@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 05/10/2013 04:39 PM, Tomas
Babej wrote:<br>
</div>
<blockquote cite="mid:518D06B1.1060904@redhat.com" type="cite">Hi,
<br>
<br>
this patcheset deals with <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://fedorahosted.org/freeipa/ticket/3602">https://fedorahosted.org/freeipa/ticket/3602</a>
<br>
<br>
See commit messages for details. <br>
<br>
Tomas <br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
Freeipa-devel mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre>
</blockquote>
<br>
I noticed during further development that logic in
interactive_prompt_callback did not follow the pre_callback
logic precisely.<br>
<br>
Fixed patches attached.<br>
<br>
Tomas<br>
</blockquote>
Hi,<br>
<br>
Patches work fine. <br>
<br>
I would suggest incorporating a fix for ticket <a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://fedorahosted.org/freeipa/ticket/3634">https://fedorahosted.org/freeipa/ticket/3634</a>
into this patchset. The issue from ticket #3634 is closely
connected to this one, and with the introduction of
prompt_param() functionality, including a fix for it would
require minimal effort. You can look at my patch (<a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://www.redhat.com/archives/freeipa-devel/2013-May/msg00297.html">https://www.redhat.com/archives/freeipa-devel/2013-May/msg00297.html</a>)
and if you think the approach is right, adjust accordingly and
incorporate it in your patchset.<br>
<br>
Other (minor) comments:<br>
<br>
* The last change in ipalib/plugins/idrange.py seems like you
wanted to fix the fact that the lines weren't properly indented
(they weren't multiples of 4). However, you also need to fix the
previous line (raise ...).<br>
* There are a lot of unused imports in ipalib/frontend.py. Since
you are already touching imports in your patch, could you clean
up the unused imports as well.<br>
<br>
<pre class="moz-signature" cols="72">--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
Freeipa-devel mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre>
</blockquote>
<br>
I addressed the minor issues. Updated patches are attached.<br>
<br>
Regarding your patch, I agree. I sent a reply to its thread.<br>
<br>
Tomas<br>
</blockquote>
ACK<br>
<br>
<pre class="moz-signature" cols="80">--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.</pre>
</body>
</html>