openerp-community-reviewer team mailing list archive

["Yannick Vaucher @ Camptocamp" <yannick.vaucher@xxxxxxxxxxxxxx>]

Review: Needs Fixing code review, no test

Hello thanks,

I see a lot of improvements here.

My remarks:

Why renaming tax_template to tax_templ? Readability counts. And it makes me wish I could auto complete those while reading the code.


It seams strange to me that you removed required=True of chart_template_id

When shouldn't it be required ? Code shouldn't rely on the view code to ensure a field is require.


An empty chart_template_id will raise errors in those methods:

name_get
_map_tax_code_template
_find_tax_codes
_find_accounts


There are some missing context:
l.1332
l.1341
l.1516
l.1646
l.2031

-- 
https://code.launchpad.net/~pedro.baeza/account-financial-tools/7.0-account_chart_update-enhanced/+merge/212074
Your team OpenERP Community Reviewer/Maintainer is subscribed to branch lp:account-financial-tools.