openerp-community-reviewer team mailing list archive

["Daniel Dico \(oerp.ca\)" <ddico@xxxxxxx>]

Hi Pedro,

Thank you for your review.

In regards with feedparser, I was under the impression that this library is required by OpenERP. Please see /openobject-server/openerpcommand/setup.py. I hope I'm not missing something here!? 

>From what I see, currency_rate_update in order to address the specific needs from a global perspective, will accumulate over time a number of different bank feeds (each with their own specific format) and, most likely, a potential user will only use one of the bank feeds... sure we can add a required library to the dependency... but it could be that users in Europe will not use the Canadian rates, while they will be forced to install the dependency. Maybe a different design could be used? Let's wait for other developers to add feeds from other banks so we can see what formats are required all over the world.

Best regards,
Daniel

> Hi, Daniel, thank you very much for the contribution.
> 
> What worries me with this new source is that you are using feedparser library.
> This library is not detected on module install, and it will only fail if you
> select this source, causing the background process to fail. The alternative is
> to put this library as dependency, but maybe is to force user to install
> another library if they are not going to use this source.
> 
> What everybody thinks about this?
> 
> Regards.
-- 
https://code.launchpad.net/~oerp.ca/account-financial-tools/add-bank-of-canada-rss/+merge/204377
Your team OpenERP Community Reviewer/Maintainer is subscribed to branch lp:account-financial-tools.