On 03.09.2013 12:00, Petr Viktorin wrote:
> On 09/02/2013 11:43 PM, Timo Aaltonen wrote:
>>
>> This fixes https://fedorahosted.org/freeipa/ticket/1887
>> and
>> https://fedorahosted.org/freeipa/ticket/2455
> 
> Thank you!
> 
>> the first three patches fix some bugs in how python is used
> 
> These look okay, I'll check when other build errors are fixed.
> 
>> fourth patch checks if dbus is already running before trying to start it
> 
> Please handle this in platform/debian/service.py.
> 
> Is this only for D-Bus or do all start() methods for Debian need this?
> If it's all of them, add it in DebianService.start.
> If it's just D-Bus you'll want to make a special service there, like
> DebianSSHService.
> 
>> fifth fixes some compilation warnings
> 
> Looks good to my eyes, perhaps a C expert can look at this one too.
> I wonder why these warnings aren't enabled in our builds, though.
> 
>> sixth finally adds the Debian platform module
> 
> Please add copyright headers to the new files.
> 
> in debian/auth.py:DebianAuthConfig.execute, you should use a dictionary
> for env:
>     env = {'DEBCONF_FRONTEND': 'noninteractive'}
> 
> You need to import ipautil to use ipautil.run in auth.py. This trips
> pylint and prevents building the code for me. Do you include pylint in
> your build procedure?
> 
> platform/debian/auth.py: Git complains about a new blank line at EOF

Ok I have the platform module patch updated, but testing is blocked
because client join fails with '401' error (authorization). This worked
fine in June, still investigating what's wrong this time..

thanks for the review!

-- 
t

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

Reply via email to