Code review comment for lp:~dreis-pt/contract-management/7.0-project_sla-dr

Revision history for this message
Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter) wrote :

PEP8 issues:
project_sla/analytic_account.py:32:9: E123 closing bracket does not match indentation of opening bracket's line
project_sla/project_sla.py:41:40: E241 multiple spaces after ','
project_sla/project_sla.py:42:9: E123 closing bracket does not match indentation of opening bracket's line
project_sla/project_sla.py:45:9: E123 closing bracket does not match indentation of opening bracket's line
project_sla/project_sla.py:81:9: E123 closing bracket does not match indentation of opening bracket's line
project_sla.py:84:9: E123 closing bracket does not match indentation of opening bracket's line
project_sla/project_sla_control.py:83:9: E123 closing bracket does not match indentation of opening bracket's line
project_sla/project_sla_control.py:149:37: E241 multiple spaces after ','
project_sla/project_sla_control.py:298:9: E123 closing bracket does not match indentation of opening bracket's line

Code issues:
project_sla/m2m.py:58:12: redundant parenthesis, if you want to make it a tuple, it should be [(5, )]
project_sla/m2m.py:73:23: redundant parenthesis, if you want to make it a tuple, it should be [(5, )]
project_sla/analytic_account.py:34:5: context is None not handled (if context is None: context = dict())
project_sla/analytic_account.py:69:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla.py:47:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla.py:60:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:85:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:104:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:127:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:164:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:300:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:307:5: context is None not handled (if context is None: context = dict())
project_sla/project_sla_control.py:317:5: context is None not handled (if context is None: context = dict())

Minor code issues:
project_sla/project_sla_control.py:112:15: would have gone with: now = dt.now().strftime(DT_FMT)
project_sla/project_issue_view.xml:60:1: Commented out code
project_sla/project_sla_control.py:104:17: Xml Tag Has Empty Body
project_sla/project_sla_control.py:104:17: Xml Tag Has Empty Body
project_sla/analytic_account.py:23:1: _logger declared but never used
project_sla/project_sla.py:24:1: no _description
project_sla/project_sla.py:65:1: no _description
project_sla/project_sla_control.py:65:20: _description not translated
project_sla/project_sla_control.py:292:20: _description not translated

Spelling:
project_sla/project_view.xml:6:37: projec -> project
project_sla/project_issue_view.xml:32:56: slak -> sla
project_sla/__openerp__.py:41:43: easilly -> easily
project_sla/project_issue_view.xml:81:19: recomputations -> re-computations
project_sla/project_sla_control.py:127:48: foun -> found

Out of curiousity, I noticed that you're using CamelCase for your class names, is there a reason for that?
Most class names in OpenERP addons are lower_case_underscore.
I personally like CamelCase, but haven't gone around to do it.

review: Needs Fixing (code review, no test)

« Back to merge proposal