Code review comment for lp:~pablocm/account-payment/7.0-account-payment-extension-fix-invoice-field

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

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.

review: Needs Fixing

« Back to merge proposal