← Back to team overview

savoirfairelinux-openerp team mailing list archive

Re: [Merge] lp:~savoirfairelinux-openerp/partner-contact-management/partner-contact-management-base_contact_add_res_bank_account into lp:partner-contact-management

 

Review: Disapprove

Description seems inaccurate to me, OpenERP already allows to manage multiple bank account per partner.

What is the plus-value when installing this module?
I suppose this is to add details on bank accounts to describe for what they should be used.
Why should I need a title_bank_account and an observation field on my res_partner_bank
Wouldn't one field be enough?
If I needed more info on res.partner.bank, I would simply add a 'description' field as on other models

title_bank_account seems useless to me as it isn't even in tree views.

Active field already exists on res.partner.bank model. By the way you overide the select=True parameter on it.



And finally this may be a matter of taste but I would name this module res_partner_bank_imp instead.


Thus sorry but I'll disaprove this one in that state.
-- 
https://code.launchpad.net/~savoirfairelinux-openerp/partner-contact-management/partner-contact-management-base_contact_add_res_bank_account/+merge/204047
Your team Savoir-faire Linux' OpenERP is subscribed to branch lp:~savoirfairelinux-openerp/partner-contact-management/partner-contact-management-base_contact_add_res_bank_account.


References