[Freeipa-devel] [PATCH] 0001 Refactor selinuxenabled check

[thierry bordaz]

On 09/26/2014 05:53 PM, Francesco Marella wrote:
>
> On 26/09/2014 17:43, thierry bordaz wrote:
>> Hello,
>>
>>     When called from set_selinux_booleans, if not selinux_enabled,
>>     you may want to 'return False' rather than 'return'.
>>     Now it looks like callers of set_selinux_booleans do not check
>>     the returned value :-)
>>
>>     thanks
>>     thierry
>>
>> On 09/26/2014 05:26 PM, Francesco Marella wrote:
>>> This should be the final one.
>>>
>>> fm
>>>
>>> On 26/09/2014 16:30, Francesco Marella wrote:
>>>>
>>>> On 26/09/2014 15:41, Petr Viktorin wrote:
>>>>>
>>>>> Hello! Thanks for the patch!
>>>>>
>>>>> The new function is not one of the platform-independent tasks, and 
>>>>> doesn't even use `self`, so you can define it as a module-level 
>>>>> helper function.
>>>>>
>>>>> But more importantly, this won't work: the blocks you are 
>>>>> replacing return from their functions. You'd need to use something 
>>>>> like:
>>>>>     if not selinux_enabled():
>>>>>         return
>>>>> instead of:
>>>>>     self.check_enabled_selinux()
>>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>
Thanks Francesco !

For me it is a ACK.

thanks
theirry
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140926/c213453c/attachment.htm>