Merge lp://qastaging/~openbmsjsc/openobject-addons/addons-extra-trunk-l10n_vn into lp://qastaging/openobject-addons/extra-trunk

Hello Phong!

Thank you for contributing to OpenERP, I think it is a very good first merge proposal!

I am not a specialist for Chart of Accounts matters, but I will give you some technical feedback first, and I will ask other colleagues that are experts with l10n modules to review the accounting details afterwards.

Here is a list of the good things and bad things I noticed (in no particular order).

Good:
- Proper use of "templates" object for most cases, without falling for the usual newcomer trap of creating real accounts, taxes, etc. in the l10n module. Good!
- Correct creation of an extra-addons branch with the addition of your module in it, this is an unintuitive part, well done!
- Correct layout of the module files, and module descriptor
- Correct licensing info
- Correct export of l10n_vn.pot and vi.po in i18n directory, nice!
- You have currently proposed the merge of l10n_vn in the extra-trunk branch, which makes sense if you want to make it available as an extra addon. This means that the community (~openerp-commiter) should probably help review your efforts. Also, if you wish to see it included in OpenERP 6.1, you can also later submit it against openobject-addons/trunk, after the cleanup.

Bad:
- You have lots of stuff that is commented out in your files (XML records, module descriptor, etc.). Comments are useful for documentation, but not to keep obsolete data around. For example you are still including the unused "account_chart.xml" file, which appears unused in your module, but takes almost 50% of the merge proposal! See guideline 2.1 in [1].
- You seem to have declared some reporting wizards in l10n_vn_account_chart_wizard_vi.xml using <wizard> but the definition of these wizard is nowhere to be found? It is not clear what you want to do exactly but in 6.0 wizards should be implemented using osv_memory objects. See [2] for details and see other modules for examples, like l10n_be.
- As a consequence of previous item, your l10n_vn.py seems quite strange and useless at the moment, as well as the related security access rights, etc.
- account.account.type's name field is not translatable anymore in 6.0 (as it was most of the time useless), so you can directly use the vietnamese names for your account types, and their translations should not be found anymore in po/pot files next time your export them.
- Unused python imports in your python files (e.g. 'import report' in __init__.py, and others in l10n_vn.py)

The rest seems good to me on a technical side.
Functionally, if you have not yet seen them, please read carefully the guidelines for l10n modules we gave during the l10n-community-sprint last year (see [3]).
Then once the small technical details above are solved, the final review by an accounting expert should be quick and there should be no remaining issue to merge your l10n_vn into official addons!

Congratulations once again for this nice work!

[1] http://doc.openerp.com/v6.0/contribute/15_guidelines/coding_guidelines.html
[2] http://doc.openerp.com/v6.0/developer/3_10_wizard/index.html#guidelines-on-how-to-convert-old-style-wizard-to-new-osv-memory-style
[3] http://pad.openerp.com/6-test-accounting-localisation-guid...

Hi Olivier,

Thank you very much for your detailed comments on improvements/clean-up that I'll need to perform. I'll make those changes ASAP, for proposal to be included in the next release of OpenERP.

I have several questions regarding this clean-up.

1. The Chart of Accounts defines skeletons of accounts, and companies can further refine/define sub-accounts for its purposes (e.g. separate different taxes schedules/rates into different sub-accounts under the same top-level accounts. Some companies just put all taxes into 1 accounts. In this case, as a template, what type these accounts (that can be optionally sub-defined) should have: the view or something else? If I use the "view" type, then in local module, where I extend/create sub-accounts, even I re-define the type of the parent account to "view", OpenERP still complains and doesn't allow me to change and create sub-accounts. Other the other hand, if I use any type other than "view", OpenERP certainly will not allow me to create sub-accounts.

2. For language localization, what should be the best practice: keep all the native language as the primary (i.e. exported in .pot file) and provide English (and any other translations) as .po files, or I should use English as the main language and have Vietnamese as a translation? The 2nd seems to be the standard approach, but as you said that the account types will not be translated ==> this doesn't work.

I'm looking forward to hearing from your feedback soon to making good progress with this module.

Thanks and best regards,

Phong.