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

[Tomas Babej]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130531/169cc605/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0055-3-Incorporate-interactive-prompts-in-idrange-add.patch
Type: text/x-patch
Size: 3364 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130531/169cc605/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0054-3-Add-prompt_param-method-to-avoid-code-duplication.patch
Type: text/x-patch
Size: 9384 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130531/169cc605/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0053-3-Remove-redundant-check-for-env.interactive.patch
Type: text/x-patch
Size: 872 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130531/169cc605/attachment-0002.bin>