Review: Needs Fixing

Hi, Pablo, thank you very much for the fix, because it's very critical to get 
this thing fix, but there are some things you can fix:

- You don't need to redefine the field invoice. This field is already defined 
in account module and if you put the same method name, it will be got called by 
inheritance. If you overwrite the field, any change in the core in the future 
will not be reflected.

- You can make the renaming with another way: instead of staying with cursor 
and user variables and change signature, you can rename all the ocurrences to 
the more standarized cr and uid. This is no problem, because overriding 
methods, positional arguments names doesn't matter.

- Why are you removing 'invoice_ids = []' line?

- Doesn't occur the same problem with arguments with _invoice_search?

Regards.
-- 
https://code.launchpad.net/~pablocm/account-payment/7.0-account-payment-extension-fix-invoice-field/+merge/203948
Your team Account Payment is subscribed to branch lp:account-payment/7.0.

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

Reply via email to