Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-23 Thread Rob Crittenden
John Dennis wrote: On 07/20/2012 12:34 PM, Petr Viktorin wrote: On 07/20/2012 05:59 PM, John Dennis wrote: A fair amount of the code in the framework is doing this now, but the install code was never cleaned up. That was left for another day, I guess that day is here. Updated. I also added

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-20 Thread Petr Viktorin
On 07/19/2012 08:01 PM, John Dennis wrote: Overall I really like the approach, good work. I have not applied and run the patch, so my comments are based only on code reading. I have just a small things which might need changing. One of the ideas in the log manager is to easily support per

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-20 Thread John Dennis
A fair amount of the code in the framework is doing this now, but the install code was never cleaned up. That was left for another day, I guess that day is here. Updated. I also added the necessary lint exception. I'm curious as to why it works that way, though. Normally, to put methods in a

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-20 Thread Petr Viktorin
On 07/20/2012 05:59 PM, John Dennis wrote: A fair amount of the code in the framework is doing this now, but the install code was never cleaned up. That was left for another day, I guess that day is here. Updated. I also added the necessary lint exception. I'm curious as to why it works that

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-20 Thread John Dennis
On 07/20/2012 12:34 PM, Petr Viktorin wrote: On 07/20/2012 05:59 PM, John Dennis wrote: A fair amount of the code in the framework is doing this now, but the install code was never cleaned up. That was left for another day, I guess that day is here. Updated. I also added the necessary lint

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-19 Thread John Dennis
Overall I really like the approach, good work. I have not applied and run the patch, so my comments are based only on code reading. I have just a small things which might need changing. One of the ideas in the log manager is to easily support per class logging (borrowed from Java). This

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-18 Thread Petr Viktorin
On 07/17/2012 10:41 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/29/2012 11:28 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/25/2012 03:00 PM, Petr Viktorin wrote: On 06/20/2012 06:15 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/04/2012 04:56 PM, Petr Viktorin wrote:

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-07-17 Thread Rob Crittenden
Petr Viktorin wrote: On 06/29/2012 11:28 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/25/2012 03:00 PM, Petr Viktorin wrote: On 06/20/2012 06:15 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/04/2012 04:56 PM, Petr Viktorin wrote: Currently, FreeIPA's install/admin scripts

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-06-29 Thread Rob Crittenden
Petr Viktorin wrote: On 06/25/2012 03:00 PM, Petr Viktorin wrote: On 06/20/2012 06:15 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/04/2012 04:56 PM, Petr Viktorin wrote: Currently, FreeIPA's install/admin scripts are long pieces of code that aren't very reusable, importable, or

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-06-25 Thread Petr Viktorin
On 06/20/2012 06:15 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/04/2012 04:56 PM, Petr Viktorin wrote: Currently, FreeIPA's install/admin scripts are long pieces of code that aren't very reusable, importable, or testable. They have been extended over time with features such as logging

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-06-25 Thread Petr Viktorin
On 06/25/2012 03:00 PM, Petr Viktorin wrote: On 06/20/2012 06:15 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 06/04/2012 04:56 PM, Petr Viktorin wrote: Currently, FreeIPA's install/admin scripts are long pieces of code that aren't very reusable, importable, or testable. They have been

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-06-20 Thread Rob Crittenden
Petr Viktorin wrote: On 06/04/2012 04:56 PM, Petr Viktorin wrote: Currently, FreeIPA's install/admin scripts are long pieces of code that aren't very reusable, importable, or testable. They have been extended over time with features such as logging and error handling, but since each tool was

Re: [Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-06-18 Thread Petr Viktorin
On 06/04/2012 04:56 PM, Petr Viktorin wrote: Currently, FreeIPA's install/admin scripts are long pieces of code that aren't very reusable, importable, or testable. They have been extended over time with features such as logging and error handling, but since each tool was extended individually,