Code review comment for lp:~manu-tiedra/openobject-addons/trunk

Hello Manu,

Sorry for the response delay, we're rather overloaded with merge proposals and the preparation of OpenERP 6.1.
Your merge proposal looks good on a technical level, but functionally we'd prefer to keep this feature in an extra module rather than modifying the logic of the original delivery module (currently the carrier_id field added by the delivery module on sale_order has only one meaning: the invoicing to be done).

I imagine it would have been quite dirty to do this in an extra module with the current state of the code, so I've just applied a quick refactoring to the delivery module, extracting a new '_prepare_shipping_invoice_line' method for preparing the creation of the shipping invoice line (see [2]). This should make it easier for you to implement what you need in an extra module.

Once your module is ready, don't hesitate to publish it on OpenERP Apps, it may be useful for other users!
See also our updated merge proposal policy and guidelines [3].

Thanks for your understanding and for your contributions!

PS: Concerning bug 625235, you'll be glad to see it was finally fixed along with bug 816138, so the patch is unnecessary now.

[1] addons rev.5395 revid:<email address hidden>
[2] http://bazaar.launchpad.net/~<email address hidden>
[3] http://bit.ly/openerp-contrib-mp