On Mon, 2012-05-07 at 14:48 +0200, Jan Cholasta wrote:
> Hi,
> 
> this patch changes the way boolean values are encoded to LDAP boolean 
> syntax. The code for encoding boolean values is moved from the Parameter 
> class to the Encoder class, where the rest of LDAP encoding takes place. 
> The patch removes encoding code from the Parameter class altogether, as 
> all LDAP encoding should be done in the Encoder class.
> 
> Unit tests show no regressions and fixes for related tickets 
> (<https://fedorahosted.org/freeipa/ticket/2039> and 
> <https://fedorahosted.org/freeipa/ticket/2616>) seem to be intact.
> 
> Honza

The patch looks ok, unit tests pass and I also did not find any
regression.

I have just one concern - with this patch, encoding is bound to native
Python type and not to our Param classes. This means that all Params
based on the same native Python type (lets say, str) will be encoded in
the same way. Are you sure that nobody would want to implement a
str-based param that has a custom LDAP encoding?

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to