openerp-community team mailing list archive

["Stefan Rijnhart \(Therp\)" <stefan@xxxxxxxx>]

Review: Needs Fixing code reading, no testing or accounting audit

Hi Tien Tran,

the diff looks very clean now! Some minor points:

l.10,38 of the diff looks like you are claiming copyright on OpenERP given the line above. Maybe remove line 9 or refer explicitely to 'this module' when claiming copyright. Looks better to say '2010-2013' (happy new year there by now!) too instead of just '2010'.

l.78-159: don't define your own account types. Use the ones from account/data/data_account_type.xml instead. If you must use your own, set the noupdate flag in the data element.

The whole account_chart.xml file is formatted irregularly, with indents of 0, 4 and 8. Please reformat with standard indentation. I believe 4 is most common in OpenERP.

l.2096,2191: setting noupdate to accounts and tax templates does not really make sense to me as they are only used at the time of chart of accounts initialization (although it seems to be common enough in other localizations).

BTW. better to invite the whole community team for review instead of a single person next time. More chance for response and I am sure other people like to keep updated too! I will invite the core team now as they only can accept your module in the official addons branch.

Best regards,
Stefan.


-- 
https://code.launchpad.net/~openerp-community/openobject-addons/gscom-70-l10n-vn/+merge/141526
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-addons/gscom-70-l10n-vn.