[Freeipa-devel] [PATCHES 0053-0055] Prompt for RID base if trusted domain SID / name is specified and vice versa

[Ana Krivokapic]

On 06/05/2013 10:01 AM, Tomas Babej wrote:
> On 05/31/2013 06:49 PM, Ana Krivokapic wrote:
>> On 05/31/2013 03:35 PM, Ana Krivokapic wrote:
>>> On 05/31/2013 12:25 PM, Tomas Babej wrote:
>>>> On 05/30/2013 02:17 PM, Ana Krivokapic wrote:
>>>>> On 05/13/2013 05:42 PM, Tomas Babej wrote:
>>>>>> On 05/10/2013 04:39 PM, Tomas Babej wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> this patcheset deals with
>>>>>>> https://fedorahosted.org/freeipa/ticket/3602
>>>>>>>
>>>>>>> See commit messages for details.
>>>>>>>
>>>>>>> Tomas
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Freeipa-devel mailing list
>>>>>>> Freeipa-devel at redhat.com
>>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>>
>>>>>> I noticed during further development that logic in
>>>>>> interactive_prompt_callback did not follow the pre_callback logic
>>>>>> precisely.
>>>>>>
>>>>>> Fixed patches attached.
>>>>>>
>>>>>> Tomas
>>>>> Hi,
>>>>>
>>>>> Patches work fine.
>>>>>
>>>>> I would suggest incorporating a fix for ticket
>>>>> https://fedorahosted.org/freeipa/ticket/3634 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
>>>>> (https://www.redhat.com/archives/freeipa-devel/2013-May/msg00297.html)
>>>>> and if you think the approach is right, adjust accordingly and
>>>>> incorporate it in your patchset.
>>>>>
>>>>> Other (minor) comments:
>>>>>
>>>>> * 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 ...).
>>>>> * 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.
>>>>>
>>>>> -- 
>>>>> Regards,
>>>>>
>>>>> Ana Krivokapic
>>>>> Associate Software Engineer
>>>>> FreeIPA team
>>>>> Red Hat Inc.
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>> I addressed the minor issues. Updated patches are attached.
>>>>
>>>> Regarding your patch, I agree. I sent a reply to its thread.
>>>>
>>>> Tomas
>>> ACK
>>>
>> 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.
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> Fixed. That actually was an unused import - when considering the code :)
>
> Thanks for catching that.
>
> Tomas
ACK for all 3 patches.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130605/877f2d71/attachment.htm>