openerp-canada team mailing list archive

["qdp \(OpenERP\)" <qdp-launchpad@xxxxxxxxxxx>]

Review: Disapprove
Hello,

can you explain the aim of this new field 'expense_include' you're adding? as far as i guess, it has exactly the same purpose than the price_include one...

Furthermore, this code 
17	+ if (partner.employee and tax.expense_include) or tax.price_include:

won't work if we havn't installed the module that add the field employee on the partner object... The proper way to do this modification on an existing function is to overwrite it in the good module and to call super().

For this reason i'm rejecting this merge proposal. We can still discuss the best way to implement your feature but first you should convince us that it's a good idea to do so (currently i have a doubt) ;-)

Thanks for your understanding,
Quentin
-- 
https://code.launchpad.net/~openerp-canada/openobject-addons/account_add_expense_include_field_to_tax/+merge/40589
Your team OpenERP Canada Team is subscribed to branch lp:~openerp-canada/openobject-addons/account_add_expense_include_field_to_tax.