Code review comment for lp:~savoirfairelinux-openerp/openupgrade-server/base_contact

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks! Looks alright to me, in principle. Two questions:

l.51 and l.78: why do you need to check if the fields exist? Seems to me this could mask potential problems in the current or future version of the code.
l.12: would it not be safer if you checked the status of the entry for 'base_contact' in the ir_module_module table? The table could theoretically belong to another module, or contain deprecated information after a deinstallation of the base_contact module.

review: Needs Information

« Back to merge proposal