Merge lp://qastaging/~dreis-pt/contract-management/7.0-project_sla-dr into lp://qastaging/~contract-management-core-editors/contract-management/7.0

Hi Daniel,

Thanks for submitting that here. A little tips: did you know you can click on "Resubmit Proposal" on the top right corner of an existing MP and change the source and destination branch ?

Using this, you conserve the historic of the discussion tha took place on the previous MP, even if source and destination branch are different. Very useful when you set a wrong destination branch or a wrong source branch or both !

Otherwise, LGTM, Thanks you.

Good to know that, thanks.

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.p...

Thanks for the thorough review, Sandy.
I just pushed the fixes, and here go some comments on it:

> PEP8 issues:

You just made me realize my pep8 configs needed some tuning.
E123 is not strict PEP8 (see E123 on https://pep8.readthedocs.org/en/latest/intro.html), and I choose not to follow it: I feel that it makes you add unnecessary indentation or extra lines.
The E241 were fixed.

> Code issues:
> project_sla/m2m.py:58:12: redundant parenthesis, if you want to make it a
> tuple, it should be [(5, )]

You are right. I ended up using [(5, 0)] because I found code in the standard addons using it like that.

> context is None not handled (if context is None: context = dict())

If context is not manipulated and is only passed through, it's unnecessary to handle the context == None case. So, I didn't fix these.

> 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.py:24:1: no _description
> project_sla/project_sla.py:65:1: no _description
> project_sla/analytic_account.py:23:1: _logger declared but never used

Yes; fixed.

> 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/project_sla_control.py:65:20: _description not translated
> project_sla/project_sla_control.py:292:20: _description not translated

Hmm, didn't find XML tags on those .py files, and I found the translation strings in the .pot file. Can you check this again?

> Spelling:

Fixed.

> Out of curiosity, I noticed that you're using CamelCase for your class names,
> is there a reason for that?

I'm using it because [PEP8](http://www.python.org/dev/peps/pep-0008/#class-names) states that "class names should normally use the CapWords convention", and I don't any reason to not follow it.

> > 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/project_sla_control.py:65:20: _description not translated
> > project_sla/project_sla_control.py:292:20: _description not translated
>
> Hmm, didn't find XML tags on those .py files, and I found the translation
> strings in the .pot file. Can you check this again?
>
Sorry, copy Paste error.
project_sla/project_sla_demo.xml:104:17: Xml Tag Has Empty Body
project_sla/project_sla_demo.xml:127:17: Xml Tag Has Empty Body

As for translating _description, if it's not done here (_description = _('...')), then it won't be translated in python code (ex: mail_thread widget -- though that one has a soon-to-be-fixed bug too https://bugs.launchpad.net/openobject-addons/+bug/1262000).
Essentially if you invoke _(self.pool.get('project.sla')._description), it will not check the pot file because it doesn't include the following for 'SLA Definition':
#, python-format

> > Out of curiosity, I noticed that you're using CamelCase for your class
> names,
> > is there a reason for that?
>
> I'm using it because [PEP8](http://www.python.org/dev/peps/pep-0008/#class-
> names) states that "class names should normally use the CapWords convention",
> and I don't any reason to not follow it.

Glad to hear that. I think I'll start using that convention too.

> project_sla/project_sla_demo.xml:104:17: Xml Tag Has Empty Body
> project_sla/project_sla_demo.xml:127:17: Xml Tag Has Empty Body

I can see that it could be neater, but strictly speaking this neither a programming nor a style issue, so I think it isn't worth the trouble.

> As for translating _description, if it's not done here (_description =
> _('...')), then it won't be translated in python code (ex: mail_thread widget
> -- though that one has a soon-to-be-fixed bug too https://bugs.launchpad.net
> /openobject-addons/+bug/1262000).

I can't find a similar example in the official addons; it seems to me that it's a temporary workaround for a bug. I would prefer to not use it unless proven to be absolutely necessary.

LGTM