On 20.10.2015 17:47, Martin Babinsky wrote:
> +    def check_domainlevel(self, api):
> +        domain_level = dsinstance.get_domain_level(api)
> +        if domain_level > MIN_DOMAIN_LEVEL:
> +            raise RuntimeError(
> +                UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE.format(
> +                    command_name=self.command_name,
> +                    min_domain_level=MIN_DOMAIN_LEVEL,
> +                    curr_domain_level=domain_level)
> +            )

NACK.

This is very very weird function because it compares two values which are not
passed as parameters, and also the parameter "api" seems to be unused.

At very least a explanatory doc string is needed, but a new name might be even
better.

Check domain level of what against what? It would be great if function name
could answer this question.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to