openerp-community team mailing list archive

["Tidiane Sy \(BAAMTU\)" <tsy@xxxxxxxxxx>]

Hello,
Thank you for your prompt and nice inputs.
I have a new revision which fixes the issues you raised.
Can you have a look please ?
Regards,
Tidiane Sy(Baamtu)
> Hello,
> 
> Thanks for this nice contribution, impressive work!
> 
> On first look, the structure of the module looks right, and seems to follow
> most of the guidelines of http://pad.openerp.com/6-test-accounting-
> localisation-guidelines
> 
> I will let my colleagues review it too before we can merge it in trunk, and
> include it in the next version.
> 
> Note that in the mean time you can already register it on apps.openerp.com to
> make it visible to everyone for 6.0. To do that you will need to push a new
> branch with only this module in it, and register it's launchpad URL on
> http://apps.openerp.com/send_branch/
> 
> Here are a few technical issues that you can fix right now:
> 
> 1) The module descriptor.
> In 6.0 and trunk, the module descriptor should be named __openerp__.py, not
> __terp__.py anymore (it still works in 6.0 but is deprecated, and may be
> dropped in trunk).
> Also, it would be nice if the description of the module included the list of
> countries that use SYSCOHADA, and perhaps a link to some online official
> reference.
> 
> 2) The copyright notice in headers should not have "All rights reserved"
> anywhere.
> This phrase was required by an old international convention, but it is now
> obsolete, and not appropriate in GPL source code. The normal "Copyright YEAR,
> AUTHOR" is sufficient and less confusing :-)
> 
> 3) "CFA" currency should be declared in 'base' and needs fixing.
> Usually we try to have currencies in the server's 'base' module (where
> res.currency is located).
> The reason is that otherwise different modules might add the same currencies
> and it will lead to duplicate entries. For example if we add it now in 'base',
> people with l10n_syscohada will have 2 different CFA currencies, leading to
> confusion. Here is how to fix this cleanly:
> 1. Make a separate merge proposal on openobject-server/trunk to add CFA to
> base_data.xml - it's a simple change, we'll merge it quickly. Be careful that
> as of 6.0, res.currency does not have a 'code' field anymore (name==code), and
> the 'rate' field should not be assigned explicitly (it's read-only), it should
> be done via a res.currency.rate entry. You might also want to have a 'symbol'
> field.
> 2. For compatibility with 6.0 and servers that don't have CFA currency, you
> can keep it in l10n_syscohada, but please change the XML ID to "base.CFA".
> This way the master currency will be in base, and it won't be created twice,
> but instead updated if it's already created by base. And it will still be
> created if base did not have it.
> 
> After fixing the above, please add a comment to this merge proposal to notify
> us, as launchpad does not notify of code changes in merge proposals by
> default.
> 
> Thanks a lot!
-- 
https://code.launchpad.net/~tsy/openobject-addons/l10n_syscohada/+merge/56816
Your team OpenERP Community is subscribed to branch lp:~tsy/openobject-addons/l10n_syscohada.