On 10/23/2015 01:51 PM, Martin Basti wrote: > > > On 23.10.2015 13:49, Tomas Babej wrote: >> >> On 10/23/2015 12:49 PM, Martin Basti wrote: >>> >>> On 23.10.2015 09:34, Martin Basti wrote: >>>> >>>> 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 >>>> >>> Patch attached. >>> O hope, I did not miss anything hardcoded. >> I think we can safely hardcode domain levels as numbers in the error >> messages. Substituting {dl} for constants.DOMAIN_LEVEL_0 in the error >> message makes little sense to me, as the value of >> constants.DOMAIN_LEVEL_0 will not ever change. > However, with substituting is easier to find occurrences of domain > levels in comments and messages in case of refactoring.
In case of refactoring of what? All the error messages containing reference to a domain level 0? :) -- 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