← Back to team overview

openerp-community team mailing list archive

Re: [Merge] lp:~openerp-community/openobject-addons/gscom-70-l10n-vn into lp:openobject-addons/7.0

 

Review: Needs Information

@Stefan: thanks a lot for the reviews!

@Tien Tran: your merge proposal looks very good to me now, thanks to Stefan's excellent review. My only remaining questions are:
 - Your module depends on both "base_iban" and "base_vat". Do you actually require these modules for Vietnam, or are those dependencies present only because you took inspiration from one of the european l10n modules? 
 - Would you mind adding a structured description for your module (in the module manifest), with a proper title (you can use RST formatting) and a module icon, to keep it consistent with the other l10n modules in 7.0? You can see a good example e.g in l10n_fr https://bazaar.launchpad.net/~openerp/openobject-addons/7.0/view/8636/l10n_fr/__openerp__.py 
   For the icon you simply have to add an image file in /static/src/img/icon.png inside your module, it does not need to be mentioned in the manifest. I'll try to post a link to the free flag icon set we used for all other modules, if I can find it. If you don't have a suitable one I can also add it when merging.

Thanks for the excellent contribution!
-- 
https://code.launchpad.net/~openerp-community/openobject-addons/gscom-70-l10n-vn/+merge/141526
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-addons/gscom-70-l10n-vn.


Follow ups

References