[Freeipa-devel] [PATCH] convert the base platform modules into packages

Timo Aaltonen tjaalton at ubuntu.com
Wed Dec 5 15:06:19 UTC 2012


On 05.12.2012 15:01, Timo Aaltonen wrote:
> On 17.10.2012 16:43, Petr Viktorin wrote:
>> On 09/21/2012 04:57 PM, Timo Aaltonen wrote:
>>> Ok, so this is the first step before we can start to rewrite bits from
>>> ipaserver/install to make them support other distros. There are no real
>>> functional changes yet.
>>>
>>> had some dependency issues installing the resulting rpm's, so didn't
>>> test the install scripts but they should work :)
>>>
>>>
>>
>> Hello,
>>
>> I recommend giving the -M flag to git format-patch, so it's easier to
>> see changes in the patch.
>>
>>
>> Your split of the fedora16 code into two modules is unfortunate: each
>> tries to import the other one, and one is the other's parent. This would
>> need special care to get working correctly.
>>
>> The best option here would probably be to put restore_context &
>> check_selinux_status into a separate submodule, so you don't need to
>> import fedora16 from services.
>>
>> Furthermore, in fedora16/__init__.py, you have:
>>      from ipapython.platform.fedora16.service import *
>> This imports everything from that module, including e.g. "redhat" or
>> "os".
>> Please avoid star imports. List all the imported names explicitly, or
>> import the module and then use qualified names.
>>
>>
>> Other than that, after a trivial rebase the patch seems to work fine on
>> Fedora. Thanks!
>
> And finally, here is version 2.
>
> fixed all the above, I think.. make-lint passes, make rpms too.

Here's v3, thanks to your rebase to an even more current master :)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-convert-the-base-platform-modules-into-packages.patch
Type: text/x-patch
Size: 35986 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121205/db10f259/attachment.bin>


More information about the Freeipa-devel mailing list