← Back to team overview

banking-addons-team team mailing list archive

Re: lp:~camptocamp/banking-addons/bank-statement-reconcile-7.0-fix-yaml-related-lep into lp:banking-addons/bank-statement-reconcile-7.0

 

Review: Needs Information

Hello, 

I'm not really happy with this workaround at least with this MP. IT should be linked to a bug report (and reciprocally).

Maybe I missed something but I do not see any yaml account_statement_base_completion, so I'm a little bit perplex.


A last anecdotic point concerns indentation on condition. 
"""
+ if (
14	+ statement.period_id
15	+ and statement.journal_id.company_id.id
16	+ != statement.period_id.company_id.id
17    ):
"""

It is really unusual
and condition is not at end of lines at least you should respect this PEP8 
sentence: 

""" Make sure to indent the continued line appropriately. The preferred place to break around a binary operator is after the operator, not before it. Some examples:

class Rectangle(Blob):

    def __init__(self, width, height,
                 color='black', emphasis=None, highlight=0):
        if (width == 0 and height == 0 and
                color == 'red' and emphasis == 'strong' or
                highlight > 100):
            raise ValueError("sorry, you lose")
        if width == 0 and height == 0 and (color == 'red' or
                                           emphasis is None):
            raise ValueError("I don't think so -- values are %s, %s" %
                             (width, height))
        Blob.__init__(self, width, height,
                      color, emphasis, highlight)

"""
Please do not start a new flame ware on community mailing list :P 



Regards

Nicolas

-- 
https://code.launchpad.net/~camptocamp/banking-addons/bank-statement-reconcile-7.0-fix-yaml-related-lep/+merge/199124
Your team Banking Addons Core Editors is subscribed to branch lp:banking-addons/bank-statement-reconcile-7.0.


References