The docstring of interactive_prompt_callback could use some tweaking, but besides that re-ACK.

Dne 21.8.2014 v 14:50 Gabe Alford napsal(a):
Hello,

         Just wondering if this needs to be re-ack'd.

Thanks,

Gabe


On Thu, Jul 31, 2014 at 7:57 AM, Gabe Alford <redhatri...@gmail.com
<mailto:redhatri...@gmail.com>> wrote:

    Okay. Sounds good. Update patch attached.


    On Thu, Jul 31, 2014 at 7:18 AM, Martin Kosek <mko...@redhat.com
    <mailto:mko...@redhat.com>> wrote:

        Ah, right. But I still think that's a too-early optimization. We
        can add this
        callback when this necessity arises. Until then, I would rather
        prefer to keep
        the code clean.

        Martin

        On 07/31/2014 03:17 PM, Gabe Alford wrote:
         > Right. The reason I added it in there is that I could see
        that in the
         > future trust_type could be more than just 'ad' (maybe 'ipa',
        'krb', etc?)
         > which at that point I'm not sure a default makes sense. So, I
        thought to go
         > ahead and add the check for future use cases so that it
        doesn't have to be
         > remembered later. However, maybe that was just a bad idea as
        right now it
         > is a pointless check?
         >
         > Gabe
         >
         >
         > On Thu, Jul 31, 2014 at 3:18 AM, Alexander Bokovoy
        <aboko...@redhat.com <mailto:aboko...@redhat.com>>
         > wrote:
         >
         >> On Thu, 31 Jul 2014, Martin Kosek wrote:
         >>
         >>> Sorry for going late in the game, just a quick question -
        why do we want
         >>> to add
         >>> this part:
         >>>
         >>> +        if trust_type is None:
         >>> +            kw['trust_type'] = self.prompt_param(self.params[
         >>> 'trust_type'])
         >>>
         >>> ? I do not see a reason for adding a special interactive
        prompt callback
         >>> for
         >>> that - trust_type has a default value "ad". CCing Alexander
        to double
         >>> check.
         >>>
         >> I also don't understand why you need to ask interactively
        for the
         >> trust_type as it defaults to non-empty value and this value
        is the only
         >> one we currently support.
         >>
         >>
         >> --
         >> / Alexander Bokovoy
         >>
         >





_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel



--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to