On 23.10.2015 13:53, Tomas Babej wrote:

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

--
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