← Back to team overview

savoirfairelinux-openerp team mailing list archive

Re: lp:~extra-addons-commiter/e-commerce-addons/7.0-sale-workflow into lp:e-commerce-addons

 

Thanks for the review,

> Thanks for the port of the module.
> Some comments below:
> 
> Please use explicit relative import.
Done

> 
> line 166: +from openerp import pooler
> does not seems to be used
Removed

> 
> Line 206 :+    except Exception, e:
> "as e" would be nicer. Also it may be useful to log the exception pgerror
> and/or pgcode
I removed the ", e" because it was not used. The traceback will be logged because there is a `log.exception()`.

> 
> You should put your authorship on automatic_workflow_job.py too.
Done

> 
> Not in the diff:
> 
> I would validate in the _prepare_write_off function the way that the write off
> type is determined with a functional guy.
Yes, it could be useful to have the advice of functional people. Note this is the same behavior as the 6.1 version.

> 
> I would not use: if not hasattr(ids, '__iter__') but if not isinstance(ids,
> (list, tuple))
Done

> 
> In stock.py there is a #TODO reimplement me. Is it still of actuality?
Yes, automatic validation of manufacturing orders is still a feature which was implemented a while ago and needs to be reimplemented. It will not be part of this MP though.

> 
> It will be nice to add a bit of documentation on the "sale.workflow.process"
> model.
Added a bit of content in the docstring.

> 
> Keep up the good job.
> 
> Regards
> 
> Nicolas
-- 
https://code.launchpad.net/~extra-addons-commiter/e-commerce-addons/7.0-sale-workflow/+merge/155920
Your team extra-addons-commiter is subscribed to branch lp:~extra-addons-commiter/e-commerce-addons/7.0-sale-method.


References