Code review comment for lp:~icsergio/stock-logistic-warehouse/improve_reordering_rules

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

__openerp__.py:
 * The description does not allow to understand what the module does
 * merge init_xml and update_xml in 'data'

l.126
from openerp.osv import orm, fields

l.128,152
inherit from orm.Model

l.150,161
remove the instantiation

l.137,144,147
context propagation is missing

l.138
why do you skip the update when you have 1 product?

l.139
unreadable query: use multiple lines

l.139
the product ids should not be substituted in the str, they must be an argument in cr.execute()

l.146
this is bug prone because you initialized res at l.135 and reuse it for each product.
Just remove the l.146 and do the write as follows:
    self.write(cr, uid, reord_rules_ids, {'product_max_qty': val[1]}, context=context)

l.158
why is 3 casted to int?

l.178-180
Instead of forcing the 'string' in the view, you better have to directly write the correct string in the python ``fields``.

On pep8 [1]
take a look on the pep8 recommandations for the whitespaces [2], 1 space after a comma or a colon (l.144,157,158...)

[1] http://www.python.org/dev/peps/pep-0008/
[2] http://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-statements

review: Needs Fixing (code review, no test)

« Back to merge proposal