openerp-community-reviewer team mailing list archive

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

Review: Needs Fixing

Ok thanks for the changes.


About the hack for required=true, please add a #FIXME comment if there is no other option. And checking chart_template_id is set in

_map_tax_code_template
_find_tax_codes
_find_accounts

Seams necessary.


To respect PEP8 80 cols I don't feel it is the right way to go. And it creates more diff than necessary. But I won't block it for that.

There are still some issue with PEP8, indentation and length of lines introduced by your changes:

l.1533
l.1903
l.1940
l.2160

l.1351 missing space after comma
-- 
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.