[Freeipa-devel] [PATCH] 78 Redo boolean value encoding

[Martin Kosek]

On Mon, 2012-05-07 at 18:49 +0200, Jan Cholasta wrote:
> On 7.5.2012 17:59, Martin Kosek wrote:
> > 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
> >
> 
> I don't think we want to do this, see 
> <http://www.redhat.com/archives/freeipa-devel/2012-April/msg00138.html> 
> (especially the ending).
> 
> Honza
> 

Ok. We may revisit this idea when the the need for custom per-Param
encoding is justified and really needed.

ACK. Pushed to master.

Martin