openerp-community-reviewer team mailing list archive

["Stefan Rijnhart \(Therp\)" <stefan@xxxxxxxx>]

Review: Disapprove

Hi Luc,

I gladly welcome the initiative of Noviat of bringing their great modules under the umbrella of the community projects!

With regards to this particular module, I must agree with Alexis that his module overlaps greatly with this module. Now before Stéphane Bidoul made a great effort to decouple the banking addons export modules from the whole bank statement import and reconciliation bit, there would have been a legitimate place for a standalone module like this. But since Alexis' SEPA export module can now also be used with only a very thin boilerplate architecture (account_banking_payment_export), I have to disagree with adopting this module too.

Looking at what this module adds is as announced, heavily biased for Belgium. This is awkward in a module that has potential use in all of Europe, but it would be perfectly alright to refactor them out into a module on top of account_banking_sepa_credit_transfer. Additionally, I see that you allow sale refunds to be selected in the payment order. Given its general usefulness, this should probably be adopted in account_banking_payment_export. Same for improving support for multi-currency environments.

I hope that my review is not discouraging you. Your input is much appreciated, but the point of the community repositories is also to combine focus and avoid double work.

Cheers,
Stefan.


-- 
https://code.launchpad.net/~luc-demeyer/account-financial-tools/7.0-account_pain/+merge/192256
Your team OpenERP Community Reviewer is subscribed to branch lp:account-financial-tools.