openerp-community-reviewer team mailing list archive

[Graeme Gellatly <gdgellatly@xxxxxxxxx>]

Review: Approve (code review, no test)

Hi,

Glad it worked for you.

I think this might be being a bit careful

+ # Reformat iline_dict so as to be compatible with what is
127	+ # accepted in res['value']
128	+ for key, value in line.items():
129	+ if isinstance(value, tuple) and len(value) == 2:
130	+ if (isinstance(value[0], int)
131	+ and isinstance(value[1], (str, unicode))):
132	+ line[key] = value[0]

You should only get a tuple if it matches those conditions anyway. Well I suppose it might be possible to get a long in pos[0], but then you would want that anyway.  But the main point here is to prefer iteritems() rather than items().  Mainly for the memory.

also consider one more change to the warning if len(lines_without_product) == len(iline_dict): raise a different warning.  Something like "This invoice does not contain any products so existing lines have not been updated.  You should update the accounts and taxes manually" - On a non product invoice that would make more sense.

But both very minor so reapprove..
-- 
https://code.launchpad.net/~akretion-team/account-invoicing/70-add-invoice_fiscal_position_update/+merge/200358
Your team Account Core Editors is subscribed to branch lp:account-invoicing.