banking-addons-team team mailing list archive

[Alexandre Fayolle - camptocamp <alexandre.fayolle@xxxxxxxxxxxxxx>]

Review: Needs Fixing code review, no test

Some nitpicking and more in-depth comments

* account_banking_payment/model/account_payment.py: 

  - there are several spaces missing in the help strings (e.g. l1507-1510 for date_prefered column 
  - btw "prefered" is spelled "preferred" in English (with two 'r'). Maybe it's still time to fix this one
  - l. 1542: missing call to tools.translate._()  for 'Payment Order Export' ?

* account_banking_payment/model/payment_order_create.py

  - I'm very suspicious of the value of _today : run this in Tokyo at 7:00 AM localtime, and _today will yield a date one day off.  This definitely requires testing with ntpd off and a compyter configured in a widely off TZ (or maybe use http://pythonhosted.org/testfixtures/datetime.html for this, if there's nothing in the OpenERP test framework)
  - I find a bit difficult to read your use of the ternary operator (with the if clause split on 2 lines, and the else clause appended at the end of the second line). If you're concerned about line lenght, I recommend using 3 lines, keeping the 'if' clause on a single line and the else clause by itself. 
-- 
https://code.launchpad.net/~therp-nl/banking-addons/ba7.0-RFR-split_off_payment_part/+merge/153680
Your team Banking Addons Team is subscribed to branch lp:~banking-addons-team/banking-addons/ba70-mig_account_iban_preserve_domestic.