Re: [Freeipa-devel] Freeipa domain levels naming
On 23.10.2015 09:31, Tomas Babej wrote: On 10/22/2015 05:49 PM, Simo Sorce wrote: On 22/10/15 11: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 Call them DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 * better readability in code Sure, but random names are not appropriate imo I'm with you guys on this, it's a good idea. Let's go with the DOMAIN_LEVEL_X naming though, it will be probably easier to remember. One thing to add to the discussion - MIN/MAX_DOMAIN_LEVEL denotes only the minimal/maximal domain level supported by the given IPA server, not the minimal/maximal domain level ever shipped by FreeIPA project. Currently, those two coincide, but in general they might be different if we ever raise the minimal level a decide to deprecate, say, domain level 0 or 1. It's a subtle but important difference. Tomas Thank you all for your opinion, I will implement DOMAIN_LEVEL_X constants and send patch. Thanks. Martin^2 -- 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
Re: [Freeipa-devel] Freeipa domain levels naming
On 10/22/2015 05:49 PM, Simo Sorce wrote: > On 22/10/15 11: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 > > Call them DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 > >> * better readability in code > > Sure, but random names are not appropriate imo I'm with you guys on this, it's a good idea. Let's go with the DOMAIN_LEVEL_X naming though, it will be probably easier to remember. One thing to add to the discussion - MIN/MAX_DOMAIN_LEVEL denotes only the minimal/maximal domain level supported by the given IPA server, not the minimal/maximal domain level ever shipped by FreeIPA project. Currently, those two coincide, but in general they might be different if we ever raise the minimal level a decide to deprecate, say, domain level 0 or 1. It's a subtle but important difference. Tomas -- 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
Re: [Freeipa-devel] Freeipa domain levels naming
On 10/22/2015 05:49 PM, Simo Sorce wrote: On 22/10/15 11: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 Call them DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 +1 * better readability in code Sure, but random names are not appropriate imo Martin^2 -- Petr Vobornik -- 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
Re: [Freeipa-devel] Freeipa domain levels naming
On 22/10/15 11: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 Call them DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 * better readability in code Sure, but random names are not appropriate imo Martin^2 -- Simo Sorce * Red Hat, Inc * New York -- 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
Re: [Freeipa-devel] Freeipa domain levels naming
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
[Freeipa-devel] Freeipa domain levels naming
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 Martin^2 -- 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
