openerp-community team mailing list archive
-
openerp-community team
-
Mailing list archive
-
Message #02058
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