Code review comment for lp:~syleam/openobject-server/trunk-dont-use-with-oids

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Looks ok, but why not drop the "WITH[OUT] OIDS" clause directly? It's deprecated and the default is off since PostgreSQL 8.1 [*] and OpenERP requires 8.3.

Also it's best to avoid any kind of whitespace alteration on lines that are not modified by the patch, because it pollutes the version history. If you really really want to change whitespace you should do it in a separate commit at least (but it's better to simply avoid it). We ask all our developers to turn all kind of "at-save" code formatting triggers, because of that ;-)

Thanks for the patch!

[*] http://www.postgresql.org/docs/8.4/static/runtime-config-compatible.html#GUC-DEFAULT-WITH-OIDS

review: Needs Fixing

« Back to merge proposal