[Freeipa-devel] [PATCH 0335] Freeipa domain levels naming

[Tomas Babej]

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? :)