On 05/31/2013 12:08 PM, Ana Krivokapic wrote:
Hello,
This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3635.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Hi, the patch itself works as it should, I have only some refactoring
comments:
1.) There already is a add_range method that is called from within
trust_add's execute()
and handles some range validation (currently only whether domain's SID
of new and
existing range are equal, my patch 67 add some more).
Your patch introduces a new method validate_range() that is called from
execute(),
which leads to some duplication of the logic, mainly two same calls to
the API (getting
the info about the old range via idrange_show)
I'd rather we keep all the validation in one place, preferably inside
the add_range method
or refactored out of it in the new method that is called only from
add_range().
2.) That being said, other parts of refactoring are nice and make code
more clear, +1.
Tomas
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
