[Freeipa-devel] [PATCH] 414-415 Move spec modifications from facet factories to pre_ops

[Ana Krivokapic]

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.


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

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130515/0ee20d35/attachment.htm>