savoirfairelinux-openerp team mailing list archive
-
savoirfairelinux-openerp team
-
Mailing list archive
-
Message #00369
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