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: ste...@therp.nl 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. _______________________________________________ Mailing list: https://launchpad.net/~openerp-community Post to : openerp-community@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community More help : https://help.launchpad.net/ListHelp
