Code review comment for lp:~lin-yu/purchase-wkfl/add_product_supplier_info

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

Hi, Lin Yu, what a good contribution! Thank you.

Let me make some suggestions:

 * Change menu name from 'Product suppliers' to 'Products by supplier'.
 * For delivery time, it's better to make the average for the group total.
 * With the new screen you have prepared, old view for entering supplier info from "Procurement" tab in "Product" form has been overwritten with two side effects:
   a) There is a new field called "Product Name", that is the product_id that must be hidden and filled automatically by context.
   b) There is no access to prices by quantities from this view when you select in Settings > Configuration > Purchases, "Manage pricelist per supplier".

On technical side, flake8 returns this output:

__init__.py:22:1: F401 'product' imported but unused
__init__.py:23:66: W292 no newline at end of file
__openerp__.py:32:80: E501 line too long (94 > 79 characters)
__openerp__.py:37:13: E203 whitespace before ':'
__openerp__.py:38:26: E231 missing whitespace after ','
__openerp__.py:38:36: E261 at least two spaces before inline comment
__openerp__.py:38:36: E262 inline comment should start with '# '
product.py:24:1: F401 'tools' imported but unused
product.py:25:1: F401 '_' imported but unused
product.py:28:1: E302 expected 2 blank lines, found 1
product.py:30:1: W293 blank line contains whitespace
product.py:31:80: E501 line too long (90 > 79 characters)
product.py:45:1: W293 blank line contains whitespace
product.py:46:13: E225 missing whitespace around operator
product.py:47:21: E203 whitespace before ':'
product.py:47:80: E501 line too long (114 > 79 characters)
product.py:48:24: E203 whitespace before ':'
product.py:48:61: E231 missing whitespace after ','
product.py:48:80: E501 line too long (96 > 79 characters)
product.py:48:83: E231 missing whitespace after ','
product.py:49:21: E128 continuation line under-indented for visual indent
product.py:49:79: E231 missing whitespace after ','
product.py:49:80: E501 line too long (106 > 79 characters)
product.py:50:28: E203 whitespace before ':'
product.py:50:65: E231 missing whitespace after ','
product.py:50:80: E501 line too long (100 > 79 characters)
product.py:50:87: E231 missing whitespace after ','
product.py:51:21: E128 continuation line under-indented for visual indent
product.py:51:79: E231 missing whitespace after ','
product.py:51:80: E501 line too long (109 > 79 characters)
product.py:54:66: W292 no newline at end of file
product_view.xml:1:2: E901 SyntaxError: invalid syntax
product_view.xml:1:14: E225 missing whitespace around operator
product_view.xml:1:29: E225 missing whitespace around operator
product_view.xml:2:9: E225 missing whitespace around operator
product_view.xml:3:5: E113 unexpected indentation
product_view.xml:3:5: E225 missing whitespace around operator
product_view.xml:3:10: E225 missing whitespace around operator
product_view.xml:5:9: E113 unexpected indentation
product_view.xml:5:9: E225 missing whitespace around operator
product_view.xml:5:19: E225 missing whitespace around operator
product_view.xml:5:60: E225 missing whitespace around operator
product_view.xml:5:73: E225 missing whitespace around operator
product_view.xml:6:13: E113 unexpected indentation
product_view.xml:6:13: E225 missing whitespace around operator
product_view.xml:6:24: E225 missing whitespace around operator
product_view.xml:6:31: E225 missing whitespace around operator
product_view.xml:6:59: E225 missing whitespace around operator
product_view.xml:6:66: E225 missing whitespace around operator
product_view.xml:7:24: E225 missing whitespace around operator
product_view.xml:7:32: E225 missing whitespace around operator
product_view.xml:7:53: E225 missing whitespace around operator
product_view.xml:7:60: E225 missing whitespace around operator
product_view.xml:8:24: E225 missing whitespace around operator
product_view.xml:8:36: E225 missing whitespace around operator
product_view.xml:8:42: E225 missing whitespace around operator
product_view.xml:9:17: E113 unexpected indentation
product_view.xml:9:17: E225 missing whitespace around operator
product_view.xml:9:31: E225 missing whitespace around operator
product_view.xml:9:55: E225 missing whitespace around operator
product_view.xml:10:17: E101 indentation contains mixed spaces and tabs
product_view.xml:10:17: W191 indentation contains tabs
product_view.xml:10:18: E113 unexpected indentation
product_view.xml:10:18: E225 missing whitespace around operator
product_view.xml:10:29: E225 missing whitespace around operator
product_view.xml:10:43: E225 missing whitespace around operator
product_view.xml:11:17: E101 indentation contains mixed spaces and tabs
product_view.xml:11:17: W191 indentation contains tabs
product_view.xml:11:29: E225 missing whitespace around operator
product_view.xml:11:51: E225 missing whitespace around operator
product_view.xml:12:21: E901 IndentationError: unindent does not match any outer indentation level

It's desirable to fix these questions.

For __init__.py, conventions that we are now agreeing say that it's convenient to put in this way:
from . import xxxx

Thank you again for all your work.

review: Needs Fixing (code review and test)

« Back to merge proposal