Code review comment for lp:~pexego/openerp-spain/6.1_account_balance_reporting_imp

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Buenas, Omar,

Muchas gracias por la mejora aportada. Cuando terminemos de refinarla, la llevaré también a la versión 7.

Te hago una primera revisión mirando código, y luego la probaré funcionalmente:

- ¿Por qué el filtro de fechas se añade al de periodos en lugar de ser excluyentes (mediante un selection o similar), como los de los informes financieros? No veo ningún caso de uso en el que haga falta filtrar por los dos a la vez. Es más, eso puede provocar confusión al usuario, ya que puede dar con una combinación de filtros de periodos y de fechas que saque datos distintos a si lo hiciera por separado (debido a las diferencias que pueden haber a veces de fechas y periodos en los asientos de proveedor).

- He visto que has mejorado bastante la convención PEP8 en el código Python, pero todavía quedan algunas cosas que corregir, por lo que te paso la salida de errores que me da un comprobador del mismo (flake8), que ya que estamos estaría bien corregir:

account_balance_reporting.py:86:9: E128 continuation line under-indented for visual indent
account_balance_reporting.py:87:9: E128 continuation line under-indented for visual indent
account_balance_reporting.py:88:9: E128 continuation line under-indented for visual indent
account_balance_reporting.py:96:9: E128 continuation line under-indented for visual indent
account_balance_reporting.py:97:9: E128 continuation line under-indented for visual indent
account_balance_reporting.py:98:9: E128 continuation line under-indented for visual indent
account_balance_reporting.py:156:46: E128 continuation line under-indented for visual indent
account_balance_reporting.py:233:9: E122 continuation line missing indentation or outdented
account_balance_reporting.py:234:9: E122 continuation line missing indentation or outdented
account_balance_reporting.py:298:49: E128 continuation line under-indented for visual indent
account_balance_reporting.py:318:49: E128 continuation line under-indented for visual indent
account_balance_reporting.py:542:9: F841 local variable 'child_ids' is assigned to but never used
account_balance_reporting_template.py:30:28: E231 missing whitespace after ','
account_balance_reporting_template.py:33:80: E501 line too long (104 > 79 characters)
account_balance_reporting_template.py:39:80: E501 line too long (119 > 79 characters)
account_balance_reporting_template.py:42:80: E501 line too long (84 > 79 characters)
account_balance_reporting_template.py:48:26: E231 missing whitespace after ','
account_balance_reporting_template.py:55:1: E302 expected 2 blank lines, found 1
account_balance_reporting_template.py:72:29: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:76:29: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:98:21: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:98:36: E225 missing whitespace around operator
account_balance_reporting_template.py:99:36: E261 at least two spaces before inline comment
account_balance_reporting_template.py:104:17: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:108:21: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:124:25: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:129:25: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:134:25: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:150:40: E127 continuation line over-indented for visual indent
account_balance_reporting_template.py:152:29: E128 continuation line under-indented for visual indent
account_balance_reporting_template.py:165:21: E128 continuation line under-indented for visual indent
account_balance_reporting_template.py:174:57: E126 continuation line over-indented for hanging indent
account_balance_reporting_template.py:195:45: E225 missing whitespace around operator
__init__.py:7:80: E501 line too long (81 > 79 characters)
__openerp__.py:7:80: E501 line too long (81 > 79 characters)
__openerp__.py:11:80: E501 line too long (80 > 79 characters)
__openerp__.py:26:9: E126 continuation line over-indented for hanging indent
__openerp__.py:26:15: E203 whitespace before ':'
__openerp__.py:27:18: E203 whitespace before ':'
__openerp__.py:28:17: E203 whitespace before ':'
__openerp__.py:29:18: E203 whitespace before ':'
__openerp__.py:30:19: E203 whitespace before ':'
__openerp__.py:36:80: E501 line too long (81 > 79 characters)
__openerp__.py:53:18: E203 whitespace before ':'
__openerp__.py:54:17: E126 continuation line over-indented for hanging indent
__openerp__.py:57:19: E203 whitespace before ':'
__openerp__.py:59:19: E203 whitespace before ':'
__openerp__.py:59:23: E201 whitespace after '['
__openerp__.py:60:21: E203 whitespace before ':'
__openerp__.py:61:17: E126 continuation line over-indented for hanging indent

Un saludo.

review: Needs Fixing (code review)

« Back to merge proposal