← Back to team overview

openerp-community team mailing list archive

Re: lp:~openerp-community/openobject-addons/stefan-therp_lp794584 into lp:openobject-addons

 

On 17-06-11 15:51, Olivier Dony (OpenERP) wrote:
> Review: Needs Information
> Hello Stefan,
>
> I agree with the idea of the bug report "wishlist", I wonder, isn't this use case sufficiently specific that it could be done in a separate module (e.g. 'ldap_import')?
> Using the "template user" system alone is perhaps sufficient for the normal case?
>
> Now, regardless of the above remark, some initial comments:
>
> - the bind() call should now support anonymous login as provided by your other merge proposal ;-)
> - DRY question: shouldn't the code of getting an ldap session, and the code of deciding/creating a user be refactored into separate functions? If we're starting to see it 2 or 3 times, it's time to think about it. Splitting these into separate functions would also allow them to be reused/extended into other module, such as perhaps an "ldap_import" one.
>
> What do you think?

Hi Olivier,

you hit the chicken right on the egg! If the basic functionality in the 
module were sufficiently broken down, I would prefer to create a new 
module for this particular use case. As it is however, I would have to 
duplicate a large part of the basic functionality in the new module (I 
had no problem doing so in the original module as this merely 
demonstrates the need for refacturing).

I will therefore reuse this branch to suggest such a breakdown in 
separate functions, and leave the added functionality for a separate 
module of our own make.

Cheers,
Stefan.

-- 
Therp - Maatwerk in open ontwikkeling

Stefan Rijnhart - Ontwerp en implementatie

mail: stefan@xxxxxxxx
tel: +31 (0) 614478606
web: http://therp.nl


https://code.launchpad.net/~openerp-community/openobject-addons/stefan-therp_lp794584/+merge/63872
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-addons/stefan-therp_lp794584.


Follow ups

References