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

2014-09-30 Thread Petr Viktorin
On 09/26/2014 06:34 PM, thierry bordaz wrote: 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

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

2014-09-26 Thread Francesco Marella
From 81d3d673944fc61de4616b5572d24719637d1d50 Mon Sep 17 00:00:00 2001 From: Francesco Marella fmare...@gmx.com Date: Fri, 26 Sep 2014 14:07:25 +0200 Subject: [PATCH] Refactor selinuxenabled check Ticket: https://fedorahosted.org/freeipa/ticket/4571 --- ipaplatform/fedora/tasks.py | 44

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

2014-09-26 Thread Petr Viktorin
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

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

2014-09-26 Thread thierry bordaz
On 09/26/2014 03:35 PM, Francesco Marella wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello, I think that if we want to keep the same previous behaviour, then if

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

2014-09-26 Thread Francesco Marella
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

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

2014-09-26 Thread Francesco Marella
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

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

2014-09-26 Thread thierry bordaz
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:

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

2014-09-26 Thread Francesco Marella
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

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

2014-09-26 Thread 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