> here, the method *returns* invoice_move_lines, and I don't see the reason for
> that as the caller already has all the invoice_move_lines it wants (unless
> `compute_invoice_totals` modifies invoice lines, which from its name it has no
> reason whatsoever to).

Well, unless I'm dumb tonight, it's just what it does: the compute_invoice_totals modifies all invoice lines. When we iterate over them, put them in the i variable (again not my fault for that local name; but again I try to make the minimal change here at least to make sure it gets in and let you make the rest if you can), and then change values of that hash, so we should return that.

Now if you see better method name or code organization please go on. Again that wouldn't be the first place in the OpenERP codebase where could easily find better names (we could speak about the foreign key name conventions and their numerous exceptions for instance)...

[...]
> Generally speaking and considering the form of your replies there's a high risk people will respond > to your tone, not your content, and I'm not sure that's for the best.

The issue is: we have been very very patient for years with Tiny with a very keen tone and the result was a huge incapacity to integrate any third party work no matter the content, such as https://code.launchpad.net/~openerp-commiter/openobject-addons/account-method-extraction/+merge/15727

So the community is tired of words, it now judges by acts only and yet for some months I believe (and see partner exchanges or community meeting or experts lists if you think that was just me, some even left recently as I said).

At the same time we invest pretty heavily on our best ERP that is still OpenERP as you see. And while OpenERP as clearly been in the lead for the last 3 years, we are a bit afraid to see that Tiny is somewhat likely to let Tryton pass within 2 years because they would make the error to dream they are ready to scale with such a poor codebase quality (even if competitors were worse it doesn't mean it's enough to scale, far from it...). That doesn't excite us at the point we would lick Tiny's boots. At this stage, the more friendly stance we can have is rather try to trigger an electroshock in Tiny's megalomania (the +500 module, Saas ready, RAD slideware)...

So don't take it personally cause we appreciate a lot the great work you are throwing in, it has been a long time... But please don't expect either people can work like mad, propose a ton of improvements, get their work largely ignored and keep smiling full of faith while competitors make their way faster... At least we continue to work in Tiny's direction...

So do you still have question or see issues with that merge? I mean yeah, it's probably far from perfect but again I try to be pragmatic: my goal is not to re-make the world here but just modularize existing stuff decently. Then I can do that elsewhere and that would provide at least much better options to extension modules for 2010 than just copy/paste those 170 lines of code... I think it has more value I do similar simple changes elsewhere where it's very much required rather than invest it all upon that one and let the rest ( https://blueprints.launchpad.net/openobject-addons/+spec/code-refactor-for-better-extensibility https://blueprints.launchpad.net/openobject-addons/+spec/refactor-old-wizards-on-osv-memory ) unchanged...