openerp-community team mailing list archive
-
openerp-community team
-
Mailing list archive
-
Message #00487
Re: [Merge] lp:~ibeardslee/openobject-addons/users_ldap-tls into lp:~openerp-community/openobject-addons/stefan-therp_lp794584
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.
References