Review: Needs Information I must agree that the features of this module, while useful, don't seem to necessarily belong together. Apart from that, the code does raise some questions with me:
- By ISP, do you mean internet service provider? - What do you mean by 'representative'? - How is code different from the regular partner reference 'ref'? Why not fill this field automatically instead of creating a new field? - Can you explain, what onchange_name does (preferably in a docstring) and how the code is assembled (preferably by refactoring the code, it would be nice if this piece of the code became a little bit more readable) You override the char field 'birthdate' from the base module by a date field. I would suspect that the installation of this module would make you 'lose' any existing birhdates in char format (orm moves the database column to birthdate_moved_nnn), and that an upgrade of the base module would move your date type column, and so on. Or am I missing something? l.207: you catch just any exception, which you should generally avoid doing. I think the whole try/catch block can be substituted by checking if 'name' is in values, right? -- https://code.launchpad.net/~savoirfairelinux-openerp/partner-contact-management/partner_isp/+merge/185095 Your team Savoir-faire Linux' OpenERP is subscribed to branch lp:~savoirfairelinux-openerp/partner-contact-management/partner_isp. -- Mailing list: https://launchpad.net/~savoirfairelinux-openerp Post to : [email protected] Unsubscribe : https://launchpad.net/~savoirfairelinux-openerp More help : https://help.launchpad.net/ListHelp
