On 22.10.2015 17:29, Martin Basti wrote: > Hello all, > > in current master branch we have mixed usage of literals 0, 1 and constants > MIN_DOMAIN_LEVEL, MAX_DOMAIN_LEVEL, and it is quite mess. > > I suggest to use names for domain levels: > > COMPAT_DOMAIN_LEVEL = 0 > PROMOTION_DOMAIN_LEVEL = 1 > UBER_NEW_FEATURE_DOMAIN_LEVEL = 2 > > MIN_DOMAIN_LEVEL = COMPAT_DOMAIN_LEVEL (=0) > MAX_DOMAIN_LEVEL = UBER_NEW_FEATURE_DOMAIN_LEVEL (=2) > > Benefits: > * ability to grep it in code > * better readability in code
I very much agree. If you want to see the reason with naked eye, see [PATCH 0086] disable ipa-replica prepare in non-zero domain levels Without additional constants we will end up with checks like: + domain_level = dsinstance.get_domain_level(api) + if domain_level > MIN_DOMAIN_LEVEL: + raise RuntimeError( Fail if domain_level is higher than MIN_DOMAIN_LEVEL? What? Even worse, bad things will happen in future when we decide to drop support for domain level 0 etc. Of course, using literals 0, 1, etc. is an option, but I do not like it very much. Again, when we drop support for a particular level we need a way to clean up the code, so named constant seems like a way to go. -- 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
