<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 03:35 PM, Ana Krivokapic
      wrote:<br>
    </div>
    <blockquote cite="mid:51A8A729.9030108@redhat.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      <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>
    </blockquote>
    After a second look, you seem to have removed to many imports from
    ipalib/frontend.py and now the doctests are failing. The import of
    `Password` from `parameters` should be put back in.<br>
    <pre class="moz-signature" cols="80">-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.</pre>
  </body>
</html>