openerp-dev-web team mailing list archive

["Jay Vora \(OpenERP\)" <jvo@xxxxxxxxxxx>]

Review: Needs Fixing
Anup,

Thanks to you and Kirti for the fix. It was really a good one.

Kindly go through these important points to improve here:

1. Variable naming convention.(Do not opt for  i,j,k,v).
2. Wrong indentation on your proposal Line 21.
3. Just a note : Whenever we use child_of operator, go for the right operant with [] of ids. This doesn't create any problem as this has been covered well recently in expression.py, but its a good practice.(Refer to line 31)
4. Does the order='id' give you a different result when order='parent_left'? (Refer to Line 31)
5. Line 61 should be location_dest_id = location. Think over it.
6. Use a shorthand operator. (Refer to line 63)
7. Improve the notification, it could be notification or warning(Line 70).
8. You don't need line 73.
9. Why do we need len(line_ids)? (Line 90).
10. Regarding last 2 for loops, I am not 100% sure whether its an optimized solution or not.
11. Regarding Line 75, are you sure we always have one inventory to check? I doubt because, we can use this wizard for multiple inventories. And its always wise to check before writing list[0].

Correct me If I missed anything.

I am investigating more in order to get the best suited fix.

Thanks.
-- 
https://code.launchpad.net/~openerp-dev/openobject-addons/6.0-bug_725908-ach/+merge/56554
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-addons/6.0-bug_725908-ach.