Code review comment for lp:~jamesmikedupont/openobject-server/openobject-server

Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

I can't say I'm fond of the way many lines are broken to fit in 80chars. Not the fitting in 80chars thing (that's great), but the way it's done e.g.

    sys.stderr.write("Running as user 'root' is a security risk, aborting.\n")

to

    sys.stderr.write("Running as user 'root'"
                     " is a security risk, aborting.\n")

I think would make more sense (and look better) as

    sys.stderr.write(
        "Running as user 'root' is a security risk, aborting.\n")

and

       db, registry = openerp.pooler.get_db_and_pool(dbname, update_module=openerp.tools.config['init'] or openerp.tools.config['update'], pooljobs=False)

to

       _db, registry = \
           openerp.pooler.get_db_and_pool(dbname, \
                                              update_module=openerp.tools.config['init'] \
                                              or openerp.tools.config['update'], \
                                              pooljobs=False)

is downright weird, why not e.g.

        to_update = openerp.tools.config['init'] \
                 or openerp.tools.config['update']
        _db, registry = openerp.pooler.get_db_and_pool(
            dbname, update_module=to_update, pooljobs=False)

this kind of wonky line-breaking seems fairly common and hampers readablility more than it helps.

review: Needs Fixing

« Back to merge proposal