On 10/23/2015 02:07 PM, Petr Spacek wrote:
On 23.10.2015 13:53, Martin Basti wrote:
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! :)
Tomas, that one day we decide to drop support for domain level 0. We will have
to hunt down all references to it and using constant for help messages etc.
might be handy because it will show up in results of grep, so we do not forget
to wipe domain level 0 from texts.
Nitpick just to make you feel that I read the patch (does not warrant a NACK):
I would rather see conditions in form of (<= or >=) instead of (< or >)
because now you have to grep for domain level +- 1 to get all references to
particular domain level :-D
+1, listen to this smart young man! Debugability++ :-)
Nevermind, this is just a nitpick.
--
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
