Review: Needs Fixing code review, no test

Nice piece of work!

Some concerns:

l.192ff I still see calls to the ORM methods without propagation of the context
l.199,1668 instead of round, I think that you should use res_currency.is_zero()

235     + for handler_func in registry.SEND_PAYMENT_FUNCS:
I don't know if this can be an issue here but you have to be cautious with this 
global registry mechanism: you always have to consider that OpenERP may import 
the python code from uninstalled modules. Meaning that you will end up with 
entries in your registry that are not part of an installed module.

241     + handler_func(self, cr, uid, cprs, context=context)
If I understand this line, you'll call the send method for each handler 
registered, so if you have handler functions for 3 banks, you'll do a call to 
the 3 banks? Couldn't it be a problem?

1643    + company = company_obj.browse(cr, uid, 1, context=context)
1 should not be hard-coded, at least use the xmlid, ideally, get it from the 
user
-- 
https://code.launchpad.net/~savoirfairelinux-openerp/banking-addons/loose-coupling/+merge/185033
Your team Savoir-faire Linux' OpenERP is subscribed to branch 
lp:~savoirfairelinux-openerp/banking-addons/loose-coupling.

-- 
Mailing list: https://launchpad.net/~savoirfairelinux-openerp
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~savoirfairelinux-openerp
More help   : https://help.launchpad.net/ListHelp

Reply via email to