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 exc

Re: [Freeipa-devel] [PATCH] 0066 Arrange stripping .po files

2012-07-20 Thread John Dennis
On 07/20/2012 12:28 PM, Petr Viktorin wrote: On 07/20/2012 05:39 PM, John Dennis wrote: Great I agree with everything you said. I'm happy to have the file list be derived from the directory contents. Are you planning on doing that in another patch? Yes, I want to do it in a new patch. It's 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 wa

Re: [Freeipa-devel] [PATCH] 0066 Arrange stripping .po files

2012-07-20 Thread Petr Viktorin
On 07/20/2012 05:39 PM, John Dennis wrote: Great I agree with everything you said. I'm happy to have the file list be derived from the directory contents. Are you planning on doing that in another patch? Yes, I want to do it in a new patch. It's a bit more complicated than it looks: creating a

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 cla

Re: [Freeipa-devel] [PATCH] 0066 Arrange stripping .po files

2012-07-20 Thread John Dennis
Great I agree with everything you said. I'm happy to have the file list be derived from the directory contents. Are you planning on doing that in another patch? FWIW the LINGUAS file was a holdover from when we first set this up based exclusively on GNU gettext suggested examples. As things h

[Freeipa-devel] [PATCH 0038] Fix two memory leaks in ldap_query()

2012-07-20 Thread Petr Spacek
Hello, this patch fixes two memory leaks in ldap_query(). Both memory leaks occurs after "non-success" queries. It effectively re-implements fix for "ldap_query can incorrectly return ISC_R_SUCCESS even when failed": http://git.fedorahosted.org/git/?p=bind-dyndb-ldap.git;a=commitdiff;h=a7cd8

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 clas

Re: [Freeipa-devel] [PATCH] 0066 Arrange stripping .po files

2012-07-20 Thread Petr Viktorin
On 07/19/2012 10:52 PM, John Dennis wrote: On 06/25/2012 07:17 AM, Petr Viktorin wrote: The translation files we currently store in Git are full of redundant information: source strings for untranslated messages, and file locations. The first causes unnecessarily huge files. The second makes dif