← Back to team overview

openerp-community-reviewer team mailing list archive

Re: lp:~camptocamp/stock-logistic-flows/7.0-add_stock_picking_priority-afe into lp:stock-logistic-flows/7.0

 

Review: Approve code review

> > picking_priority.py:
> >  * l.123 you're not checking if ids is an int or a list, this could lead to
> a
> > problem with search and an error on l.137
> 
> I respectfully disagree. The name of the parameter implies that expected type
> is a list. Adding such tests only clutters the code imo. Calling the function
> with a single integer would be a bug in the calling function.

I agree with Alexandre. Only some methods of the ORM (write, unlink, browse, name_get, perm_read) do accept both, which is a regretted choice today AFAIK, but can no longer be changed to keep the backward compatibility. But all the other methods do not have to support boths.

LGTM, thanks
-- 
https://code.launchpad.net/~camptocamp/stock-logistic-flows/7.0-add_stock_picking_priority-afe/+merge/197206
Your team Stock and Logistic Core Editors is subscribed to branch lp:stock-logistic-flows/7.0.


References