Review: Needs Fixing

Maxime,

you merged this a bit too fast for my taste, two hours after the resubmit. 
Please allow a couple of days for other people to react.

This module deletes the original BOM report, and then redefines it. I assume 
that it contains a copy of the original report plus the additional fields that 
this module defines. I would advocate two changes;

Instead of deleting the report, just set the rml location on the existing 
report to the rml file in this module. Otherwise, this will impact models that 
refer to the original report, such as an email template.

The change of the location of the rml template should be stated in the module 
description.

If you agree to these changes, it is probably easier to submit a patch branch 
than to uncommit this hasty merge.


-- 
https://code.launchpad.net/~savoirfairelinux-openerp/openerp-manufacturing/add-industrial-design-bom/+merge/161126
Your team Savoir-faire Linux' OpenERP is subscribed to branch 
lp:~savoirfairelinux-openerp/openerp-manufacturing/add-industrial-design-bom.

-- 
Mailing list: https://launchpad.net/~savoirfairelinux-openerp
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~savoirfairelinux-openerp
More help   : https://help.launchpad.net/ListHelp

Reply via email to