openerp-community team mailing list archive
-
openerp-community team
-
Mailing list archive
-
Message #00372
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