Review: Needs Fixing code review
A global remark: OpenERP uses the word 'Journal Entries' for the moves, I think
you should use the same word (for the UI strings).
A few typos:
in __openerp__.py
s/allow to groupe/allows to group/
in __init__.py
s/import statement/from . import statement/
in statement.py
s/AccountStatementProfil/AccountStatementProfile/
Indentation + proposition of string/help message:
_columns = {
'one_move': fields.boolean(
'Group Journal Items',
help="Only one Journal Entry will be generated on the "
"validation of the bank statement.")
}
l.190 if st.journal_id.default_debit_account_id is False, you will get
st_line.account_id, is it the behavior you want ? (generally, using
...and...or... is considered a bad thing, but seems you used it intentionally
to fallback on the st_line.account_id)
l.195ff just a little thing but while you are at it: the indentation should be
vertically aligned
l.228 Will crash if a statement has 0 lines. It deserves an "assert
st.line_ids" if this situation can't happen (in usual condition), or a check if
the situation may happen (in usual condition)
l.229 Use != instead of <>
A few pep8 warnings when using a tool like flake8/pep8.
l.260: priority useless (default value is 16)
l.261: deprecated 'type' in view
Thanks!
--
https://code.launchpad.net/~akretion-team/banking-addons/bank-statement-reconcile-7.0-one-move/+merge/197769
Your team Banking Addons Core Editors is subscribed to branch
lp:banking-addons/bank-statement-reconcile-7.0.
--
Mailing list: https://launchpad.net/~banking-addons-team
Post to : [email protected]
Unsubscribe : https://launchpad.net/~banking-addons-team
More help : https://help.launchpad.net/ListHelp