Code review comment for lp:~therp-nl/account-financial-tools/6.1-account-purchase-default

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Ronald,

here are my nits:

l.17,42 The module name is not very clear. How about supplier_invoice_default_account?

l.48 Please set 'Therp B.V.' as author, as this prevents multiple 'Therp' entries showing up in the authors sesction of the Apps site and allows people to filter on all our addons by a single click.

l.59..62 init_xml and update_xml are deprecated. Please use the 'data' directive.

l.102 osv.osv is deprecated. Use orm.Model

l.108,109 use "context.get('partner_id')" and "context.get('type')"

l.126 typo 'in_refurd' -> 'in_refund'

l.112,130 You could consider using read() instead of browse(), as it adds less overhead and you only read a single field.

l.203,218 'data' tag at this level no longer necessary from v6.1 on.

review: Needs Fixing

« Back to merge proposal