Merge lp://staging/~mikel-martin/purchase-wkfl/6.1-purchase_landed_costs into lp://staging/~purchase-core-editors/purchase-wkfl/6.1

Hello, what do you mean by "purchase_landed_cost from c2c-rd-addons fixed"?
Is it an extraction of the module from c2c-rd-addons, with some fixes?
My concern is that there is only 1 revision, so we can't see what is part of the extraction and what are your fixes.

Yes, that is.

The costs were not properly calculated in the stock picking if there was more than one order line. This fix has already been done in the v7 of this module.

If you have a look at the MP in c2c-rd-addons https://code.launchpad.net/~mikel-martin/c2c-rd-addons/6.1/+merge/199583 you'll see the diff, it is quite clear what the problem was.

Lots of PEP8 issues to fix

osv.osv is depricated in 6.1, use orm.Model
functions that have context as a parameter need to start with:
    if context is None:
        context = dict()
Many browse calls should have context=context in their parameters, the same applies to other self calls which include context=None in their parameters

Code issues:
purchase_landed_costs/purchase.py:224:65: should be context=context

Minor code issues:
purchase_landed_costs/product.py:23:1: should be importing from openerp.osv
purchase_landed_costs/product.py:24:1: unused import
purchase_landed_costs/product.py:34:132: unnecessary backslash
purchase_landed_costs/product.py:41:131: unnecessary backslash
purchase_landed_costs/purchase.py:23:1: should be importing from openerp.osv
purchase_landed_costs/purchase.py:24:1: should be importing openerp.decimal_precision
purchase_landed_costs/purchase.py:25:1: unused import
purchase_landed_costs/purchase.py:229-236: could be a single statement (val = {'product_id': order_cost.product_id.id, ... }

Spelling:
purchase_landed_costs/stock.py 34:18: costss -> costs
purchase_landed_costs/stock.py 49:18: costss -> costs
purchase_landed_costs/stock.py:52:15: distrubution -> distribution
purchase_landed_costs/stock.py 65:18: costss -> costs
purchase_landed_costs/stock.py 124:18: costss -> costs
purchase_landed_costs/product.py:44:155: catgory -> category
purchase_landed_costs/purchase.py:68:18: costss -> costs
purchase_landed_costs/purchase.py:102:18: costss -> costs

Thanks for your review.

Which tool do you use for the code review it would com in handy.

There isn't really a tool

pep8-python2 for pep8
pycharm for code inspection and spelling
the rest is by hand

Fixed the code issues

PEP8:
purchase_landed_costs/__init__.py:3:2: W291 trailing whitespace
purchase_landed_costs/__init__.py:19:75: W291 trailing whitespace

purchase_landed_costs/__openerp__.py:30:40: W291 trailing whitespace
purchase_landed_costs/__openerp__.py:42:27: E202 whitespace before ']'

purchase_landed_costs/product.py:19:13: E128 continuation line under-indented for visual indent
purchase_landed_costs/product.py:31:13: E128 continuation line under-indented for visual indent

purchase_landed_costs/purchase.py: 37 PEP8 errors
purchase_landed_costs/stock.py: 25 PEP8 errors

Code:
purchase_landed_costs/purchase.py 62:9: you have the same two lines twice
purchase_landed_costs/purchase.py 82:9: you have the same two lines twice
purchase_landed_costs/purchase.py 280:57: should be context=context
purchase_landed_costs/purchase.py 294:57: should be context=context

purchase_landed_costs/stock.py:20:21: missing context=context
purchase_landed_costs/stock.py:38:21: missing context=context
purchase_landed_costs/stock.py:40:15: distrubution -> distribution
purchase_landed_costs/stock.py:62:21: missing context=context
purchase_landed_costs/stock.py:73:21: missing context=context
purchase_landed_costs/stock.py:113:21: missing context=context
purchase_landed_costs/stock.py:128:21: missing context=context
purchase_landed_costs/stock.py 143:18: costss -> costs
purchase_landed_costs/stock.py:144:21: missing context=context
purchase_landed_costs/stock.py:159:21: missing context=context
purchase_landed_costs/stock.py:173:21: missing context=context
purchase_landed_costs/stock.py:188:21: missing context=context

Hi sorry, I disagree with

> functions that have context as a parameter need to start with:
> if context is None:
> context = dict()

As per openerp coding guidelines this is not necessary if you are only passing context downstream and not accessing it anywhere in your function. Convention for some maybe, but not required and ultimately just adds unneeded LOC.

https://doc.openerp.com/6.0/contribute/15_guidelines/coding_guidelines_framework/#the-infamous-context

Yet another try with some more fixes.

Still about 38 PEP8 errors, run the following command to see them all.
$ find purchase_landed_costs/ -name \*.py -exec pep8-python2 --show-source --ignore E501 {} \;
or
$ pep8-python2 --show-source --ignore E501 purchase_landed_costs/purchase.py
$ pep8-python2 --show-source --ignore E501 purchase_landed_costs/stock.py

Yet another try.

Does anyone know why eclipse doesn't complain about E121 and E127?

I've configured eclipse to use pep8 in this way http://stackoverflow.com/questions/399956/how-to-integrate-pep8-py-in-eclipse/8532188#8532188

Your code passes my PEP8 check.

One more thing
purchase_landed_costs/stock.py:3:1: F401 'itemgetter' imported but unused

Hi,

Thanks for the contribution. I approve this MP's, but I'm also in-line with Guewen. You should have sued the extraction tools located here : https://launchpad.net/bazaar-extractor to replay the commit of this module in that branch. This way we don't lose all the commit. But well, as it is on a old 6.1 branch I think it's ok not to block this MP for that.

Note that I ported the same module and add some improvements for the 7.0 serie.

Regards,

Joël