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.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-community
More help : https://help.launchpad.net/ListHelp
