<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 06:49 PM, Ana Krivokapic
      wrote:<br>
    </div>
    <blockquote cite="mid:51A8D498.3090901@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 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>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <br>
      <pre wrap="">_______________________________________________
Freeipa-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<a 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>
    Fixed. That actually was an unused import - when considering the
    code :)<br>
    <br>
    Thanks for catching that.<br>
    <br>
    Tomas<br>
  </body>
</html>