Merge lp://staging/~schampagne/openerp-manufacturing/mrp_bom_product_details into lp://staging/openerp-manufacturing

Status: Merged
Merged at revision: 3
Proposed branch: lp://staging/~schampagne/openerp-manufacturing/mrp_bom_product_details
Merge into: lp://staging/openerp-manufacturing
Diff against target: 154 lines (+134/-0)
4 files modified
mrp_bom_product_details/__init__.py (+23/-0)
mrp_bom_product_details/__openerp__.py (+35/-0)
mrp_bom_product_details/mrp_bom_product_details.py (+34/-0)
mrp_bom_product_details/mrp_bom_product_details.xml (+42/-0)
To merge this branch: bzr merge lp://staging/~schampagne/openerp-manufacturing/mrp_bom_product_details
Reviewer Review Type Date Requested Status
Alexandre Fayolle - camptocamp Approve
Guewen Baconnier @ Camptocamp no test, review Approve
Maxime Chambreuil (http://www.savoirfairelinux.com) Approve
Review via email: mp+143791@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903) :
review: Approve
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

As this is for version 7.0, it would be better to:

Inherit from orm.Model instead of osv.osv, because osv.osv is deprecated
    from openerp.osv import fields, orm
    from openerp.tools.translate import _

    class mrp_bom(orm.Model):

And you no longer need to instantiate your model at line 107

The lines 103, 104 are very long...

So it would be great if you can change that.

Expect that, that looks good to me

review: Approve (no test, review)
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

LGTM

review: Approve
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

> LGTM

Sorry, the comment went out too soon.

Please change the base class to Model and remove the class instanciation, as pointed out by guewen

4. By root <root@prog01>

removed deprecated code, fixed typo, improved code readability

Revision history for this message
samuel champagne (schampagne) wrote :

Requestion modifications have been made.

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

I tried to do the merge but the target branch is empty (no revisions) and I got:

bzr: ERROR: Merging into empty branches not currently supported, https://bugs.launchpad.net/bzr/+bug/308562

Is it intended that lp:openerp-manufacturing is empty?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches