Code review comment for lp:~elbati/account-consolidation/adding_account_parallel_currency_7
Revision history for this message
| Stefan Rijnhart (Opener) (stefan-opener) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
67d34a1
(Get the code!)
This has been the oldest proposal on the community projects for some time now, maybe because of the relatively rare business case, and the large amount of code impacting the core of the accounting system. So I'll have ago. I think I can understand the particular business case of this module, and the code does look good. Tests are provided too, so I think this should be fine.
It would be nice to know whether this module has already been used in production by now?
One nit:
l.174..185 + l.535..536 is a bit tiresome. Can you make this a list comprehension or at least iterate over a list of fields?