On 05/16/2013 02:01 PM, Petr Vobornik wrote:
On 05/15/2013 04:02 PM, Ana Krivokapic wrote:
On 05/15/2013 12:09 PM, Petr Vobornik wrote:
[PATCH] 414 Move spec modifications from facet factories to pre_ops
-------------------------------------------------------------------

Spec modifications in factories makes inheritance and extensibility
more difficult.

Moving them to pre_ops allows modification of their output by other
pre_ops.

https://fedorahosted.org/freeipa/ticket/3605

[PATCH] 415 Unite and move facet pre_ops to related modules
-----------------------------------------------------------

Facet pre_ops defined in ./facet module were moved to modules where
facet are actually defined. Moved pre_ops were united with the ones
defined for the facets in these modules.

The move simplifies module dependencies - there is no reason to have
general facet module dependent on specialized facet modules.

Pre_ops uniting makes the code simpler.

https://fedorahosted.org/freeipa/ticket/3605



A minor nitpick - in patch 415:

+        if (indirect_members) {
+            if (indirect_members.indexOf(spec.other_entity) > -1) {
+                return true;
+            }
+        }

This construct:
if (A) {
     if (B) {
         do_something();
     }
}

Can be replaced with:
if (A && B) {
     do_something();
}

The second option is more readable, and avoids nested ifs.

Changed:
-        if (indirect_members) {
-            if (indirect_members.indexOf(spec.other_entity) > -1) {
-                return true;
-            }
-        }
-        return false;
+        var has_indirect = !!(indirect_members &&
indirect_members.indexOf(spec.other_entity) > -1);
+        return has_indirect;


Updated patch attached.



I have found no other issues, so ACK from me.

--
Regards,

Ana Krivokapic

Pushed to master, ipa-3-2.
--
Petr Vobornik

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

Reply via email to