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
« Back to merge proposal
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 code_template
_map_tax_
_find_tax_codes
_find_accounts
There are some missing context:
l.1332
l.1341
l.1516
l.1646
l.2031