Review: Resubmit
Hi Olivier,

glad you like our work! Thank you for your valuable comments. I have tried to 
honour all of them in my latest commit. Additional comments:

> - l.94, l.278, l.339: not sure if this was done on purpose, but when logging
> from within an except block, you can simply use logger.exception('foo') to
> automatically log the full exception info and trace

Did not think that the trace is very interesting in these cases, as the 
exceptions most likely occur due to a configuration error or an error on the 
side of the LDAP server. I did change the code so as to leave the formatting to 
the logger though.

> - l.183: the explicit test on empty passwords at the start of login() is
> really needed

Ouch, that hurts :-( Thank you for saving me there! I put the check right in 
the heart of the authorization method of the ldap class, so there is no 
escaping it now.

Regards,
Stefan.

-- 
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     : [email protected]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp

Reply via email to