banking-addons-team team mailing list archive

Thread
Date

Re: [Merge] lp:~akretion-team/banking-addons/bank-statement-reconcile-7.0-add-completion-label into lp:banking-addons/bank-statement-reconcile-7.0

To: Sébastien BEAU - http://www.akretion.com <sebastien.beau@xxxxxxxxxxxx>
From: Nicolas Bessi - Camptocamp <nicolas.bessi@xxxxxxxxxxxxxx>
Date: Tue, 04 Feb 2014 13:11:36 -0000
In-reply-to: <20131204172505.32517.22592.launchpad@ackee.canonical.com>
Reply-to: mp+197758@xxxxxxxxxxxxxxxxxx
Sender: bounces@xxxxxxxxxxxxx

Review: Needs Fixing code review, no test

Hello, 

Thanks for the patch.

There is PEP8 here and there, in all py files, a lynter should do the trick.

in statement.py there is some unused variables 
sub_query
st_line_obj = self.pool.get('account.bank.statement.line')
label_obj = self.pool.get('account.statement.label')


ErrorTooManyPartner is used but not imported/defined
from openerp.addons.account_statement_base_completion.statement import ErrorTooManyPartner

in SQL this seems odd.
ILIKE %s || l.label || %s Why not use posix regex support?

Also the sql query can be simplifed using a unique join and a with statement.


Regards

Nicolas


-- 
https://code.launchpad.net/~akretion-team/banking-addons/bank-statement-reconcile-7.0-add-completion-label/+merge/197758
Your team Banking Addons Core Editors is subscribed to branch lp:banking-addons/bank-statement-reconcile-7.0.