openerp-community-reviewer team mailing list archive

[Joël Grand-Guillaume @ camptocamp <joel.grandguillaume@xxxxxxxxxxxxxx>]

Review: Needs Fixing code review, no tests

Hi Lin,


Thanks for the proposal ! A few remarks here:

 * Line 163,170, 179 : please split the lines, too long. You can achieve that using intermediate variable name
  e.g. 

product_limit = self.pool.get('res.partner').read(cr,uid,context['supplier_id'],['supplier_product_limit'])['supplier_product_limit']

Can be:

part_obj = self.pool.get('res.partner')
supplier_id = context['supplier_id']
product_limit = part_obj.read(cr,uid,supplier_id,['supplier_product_limit'])


 * Line 166: Any reason not to use the ORM here ? I think you should.

 * Line 179: The help tool tip is confusing for me. Took me a while to get the purpose of the field.. I suggest something more like : 

Check that box if you want to only search in products catalog of this supplier when fulfilling a purchase order for him. Otherwise, all products catalog will be available as normal.

 " Line 66: The description of the module should be a bit more precise I think. Refer to the point above to better explaine what "Enables/Disables the supplier control in Purchase" means.


Have a nice day,

Joël


-- 
https://code.launchpad.net/~lin-yu/purchase-wkfl/add_purchase_control_suuplier/+merge/180795
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.