Code review comment for lp:~vauxoo/openobject-addons/trunk-l10n_co

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Thanks for fixing the dependency, and for contributing yet another Chart of Accounts! Impressive work, first time I see one with such extensive documentation of the legal background.

Minor details:

1. The way we trigger the installation wizard for l10n_* modules has been simplified in trunk at revision [1]. You can now simply change the state of the shared installation wizard "todo" to "open" in order to trigger it again. It should be done in a noupdate data block to avoid re-triggering it at each update. I can fix that while merging.

2. I will have to remove the empty i18n subdirectory when merging, to avoid issues with the way we manage the translation exports. I'd rather not have any i18n directory when translations are disabled for a module (and I think they should not be enabled for l10n_co, right?)

3. Don't mind the other empty directories, I suppose they make sense if you plan to extend the module. It's a bit surprising that the Chart of Account data is not in the "data" subdirectory, though, since there is one, but it doesn't really matter ;-)

If you're ok with points 1 and 2 above, I can do them myself when merging.

Thanks!

[1] http://bazaar.launchpad.net/~openerp/openobject-addons/trunk/revision/7841

« Back to merge proposal