openerp-community team mailing list archive

["Olivier Dony \(OpenERP\)" <odo@xxxxxxxxxxx>]

Review: Needs Fixing

Hi Geert,

Thanks a lot for your work, and sorry we were not able to reply earlier.

You did well for the publishing of your patch, and the diff shown on this merge proposal simply indicates conflicts in the nl_BE.po file because of the way Launchpad is managing the translation files in our Bazaar branches[1]. There's no easy way to avoid that when a patch involves PO files (more on that below).

I see now two main problems with the translation of the l10n_be Chart of Accounts, and you have noticed the first one during your implementation. These are:

1/ The l10n_be module is a bit special in the fact that it executes the CoA installation wizard directly when the module is first installed, instead of triggering the CoA wizard in the UI *after module installation*, like other l10n modules do. Combined with the fact that PO files are loaded as the last step in a module installation[2], this prevents l10n_multilang from properly installing the translations for the Chart of Accounts.
Many solutions are possible to circumvent this (including removing the YML file as you did), but one of them might kill two birds with one stone[3], so read on about the next issue.

2/ Launchpad is managing the PO files of all official modules by committing daily all the translations submitted by the community using the translation interface, based exclusively on the reference POT file. This means that any extra translation you put in nl_BE.po will be discarded at the next synchronization as obsolete terms (as they're not in the POT). And we can't put the Chart of Accounts terms in the l10n_be.pot template because that would defeat the very purpose of using l10n_multilang, as you know. So your current patch would be quickly broken if we merge it as it is.
If you wonder about it, l10n_ch does not have this problem because the translations were never enabled for it in Launchpad, for some reason. This is probably not 100% correct because there may be generic terms or error messages added by l10n_ch (for example inside reports[4]) that will not be translated/translatable in other languages. 


There are various possible solutions for each of these issues, but one would fix them both at once: moving the Dutch translations of the Chart of Accounts from i18n/nl_BE.po to a separate PO file (e.g. l10n_be/nl_BE_chart.po), and loading it programmatically during the installation, for example via an extra step in the existing YML script. By making it a separate PO file outside of the i18n/ directory you make sure Launchpad will not touch it, and by loading it programmatically you decide at which point of the installation it will be processed. The YML code should be fairly simple, it only needs to call `openerp.modules.get_module_resource('l10n_be', 'i18n', 'nl_BE_chart.po')` to obtain the full path to the PO file, and then call `openerp.tools.translate.trans_load(cr, filename, 'nl_BE')` to process it. You can also remove all terms that are not related to the translation of the Chart of Accounts in your nl_BE_chart.po file, to avoid duplicating the default translations from the i18n directory.

If you have trouble figuring out that bit of YML code please say so, as YML is tricky to get right. I can give you a working code snippet, but I did not have time to write and test that yet.


Let me know if the remarks above make sense to you and if you need any help with them. After applying this solution (and reverting the changes that are not relevant anymore) your merge proposal should not have any conflicts anymore and the patch should be small and simple to review and merge in the official branch.
Note that this merge proposal will be automatically updated whenever you push more commits in the existing branch.

Thank you so much for your patience, your dedication, and your excellent contribution!



[1] Read more about that in our documentation: https://doc.openerp.com/7.0/contribute/07_improving_translations/
[2] This is for a very good reason: technically we can't load translations for fields belonging to records that do not exist in the database yet, so all data files (be them CSV, XML or YML) must be loaded *before* the PO files.
[3] Of twee vliegen in één klap ;-)
[4] Reports can be translated in the language of the intended recipient, for instance when you print an invoice the labels/titles are translated in the language you set on the invoiced customer. A l10n module may contain reports that are intended for customers in foreign languages, so translating those using the generic OpenERP system actually makes sense.
-- 
https://code.launchpad.net/~openerp-community/openobject-addons/gjanssens_l10n_be_nl/+merge/165071
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-addons/gjanssens_l10n_be_nl.