Code review comment for lp:~ibeardslee/openobject-addons/users_ldap-tls

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Ian,

thanks for resubmitting this. I have not seen this prerequisite branch functionality of bzr in action before, but it appears to be working pretty well apart from the phantom addition of 'hr_payroll/i18n/vi.po' that none of us commited in these branches (but that was added to the target branch in the meantime).

I am generally happy with your code changes and documentation, and have only one request. With regards to ldap.conf, you mention its default location and the fact that it may vary between distributions. It may be more correct to refer to its man page, which starts by explaining which files and locations are searched for LDAP defaults, in what order and how this can be manipulated using environment settings. The Sphynx syntax for this is

:manpage:`ldap.conf{5}`

Cheers,
Stefan.

review: Needs Fixing

« Back to merge proposal