openerp-community-reviewer team mailing list archive

["Holger Brunn \(Therp\)" <hbrunn@xxxxxxxx>]

Review: Needs Fixing code review

I'd also prefer OpenERP's API, so +1 for safe_eval.

But more picking on this line: The '&' makes your code crash for an empty attachment domain ('[]') and adds no benefit I can see. Do I overlook some benefit?

Generally, I think it's safer to use osv.expression.normalize and osv.expression.combine where applicable.

Further: What purpose does conditional_attachment_ids on ir.attachment serve?
-- 
https://code.launchpad.net/~therp-nl/server-env-tools/7.0-add_email_template_conditional_attachment/+merge/202740
Your team Server Environment And Tools Core Editors is subscribed to branch lp:server-env-tools.