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?
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. /pep8.readthedo cs.org/ en/latest/ intro.html), and I choose not to follow it: I feel that it makes you add unnecessary indentation or extra lines.
E123 is not strict PEP8 (see E123 on https:/
The E241 were fixed.
> Code issues: sla/m2m. py:58:12: redundant parenthesis, if you want to make it a
> project_
> 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: sla/project_ sla_control. py:112: 15: would have gone with: now = ).strftime( DT_FMT) sla/project_ issue_view. xml:60: 1: Commented out code sla/project_ sla.py: 24:1: no _description sla/project_ sla.py: 65:1: no _description sla/analytic_ account. py:23:1: _logger declared but never used
> project_
> dt.now(
> project_
> project_
> project_
> project_
Yes; fixed.
> project_ sla/project_ sla_control. py:104: 17: Xml Tag Has Empty Body sla/project_ sla_control. py:104: 17: Xml Tag Has Empty Body sla/project_ sla_control. py:65:20: _description not translated sla/project_ sla_control. py:292: 20: _description not translated
> project_
> project_
> project_
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.