← Back to team overview

openerp-community-reviewer team mailing list archive

Re: [Merge] lp:~acsone-openerp/account-financial-tools/account_partner_required-sbi into lp:account-financial-tools

 

Hello Lorenzo,

About the constraints. This module is heavily inspired by account_analytic_required which used the same technique. I must confess I did not question the approach. Initially account_analytic_required was working mainly on the vals dictionary so it would have been faster than constraints. While working on account_partner_required I identified a bug in account_analytic_required which required a refactoring [1] which possibly rendered the optimization less obvious. Now since there are really two constraints to be checked based on the policy, the current approach is probably still slightly faster.

Would you agree to keep the current approach and postpone a possible performance test / refactoring with constraints to another MP?

Regarding yml vs unittest, it's only a matter of personal taste in this case. I notice the unittest2 framework seems better documented [2]. But hey, at least I have tests, right :)


[1] https://code.launchpad.net/~acsone-openerp/account-analytic/account_analytic_required-test_suite-sbi
[2] https://doc.openerp.com/trunk/server/05_test_framework/
-- 
https://code.launchpad.net/~acsone-openerp/account-financial-tools/account_partner_required-sbi/+merge/216442
Your team Account Core Editors is requested to review the proposed merge of lp:~acsone-openerp/account-financial-tools/account_partner_required-sbi into lp:account-financial-tools.


References