openerp-community-reviewer team mailing list archive

[Alexandre Fayolle - camptocamp <alexandre.fayolle@xxxxxxxxxxxxxx>]

Review: Needs Fixing code review, no test

* in select_or_create_prodlots_from_interval: 

The following lines

282	+ prodlot_seq = ['%%s%%0%dd%%s' % number_length % (
283	+ prefix, current_number, suffix) for current_number in
284	+ range(first_number, last_number+1)
285	+ ]

should be rewritten using '%s%0*d%s' % (prefix, current_number, number_length, suffix)  (see http://docs.python.org/2.7/library/stdtypes.html#string-formatting-operations)

I also suggest testing that the interval limits are > 0

* spell check: s/splitted/split/ everywhere

* product_serial_sale_stock/sale_stock.py : the code is not calling super()._create_pickings_and_procurements and will therefore break any addons which overloads this method. If you really need to reimplement the method without playing the super game, this needs at least to be proeminently documented in the module description. And I think monkey patching the official addon implementation would be less disruptive to other addons. 

* there are no automated tests for the new feature. 


-- 
https://code.launchpad.net/~akretion-team/stock-logistic-flows/70-product_serial-plus-plus/+merge/195144
Your team Stock and Logistic Core Editors is subscribed to branch lp:stock-logistic-flows.