Review: Needs Fixing
Hi Ian,

the code looks good, but you can prevent the duplicity by changing the 
res_company_ldap.connect() method itself instead of triggering STARTTLS on the 
result of this method twice (diff lines 45:46 and 54:55).

Personally, I would not put this detail in the tree view but that is a matter 
of taste.

If I merge this branch with my code, it will be less obvious that you 
contributed this feature once my branch gets merged with 
openobject-addons/trunk. Therefore, I presume that you will want to fire 
another merge request with the official branch and not have it merged here.

Thanks,
Stefan.

-- 
https://code.launchpad.net/~ibeardslee/openobject-addons/users_ldap-tls/+merge/71131
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