← Back to team overview

openerp-community team mailing list archive

Re: [Merge] lp:~openerp-community/openobject-addons/gscom-70-l10n-vn into lp:openobject-addons/7.0

 

Review: Resubmit

Hi Stefan,

I have re-edited my module follow by your comment. Please help me review my changes. Hope it is OK now.

Thank you very much.

Tien Tran

> 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.


References