account-financial-reporting icon indicating copy to clipboard operation
account-financial-reporting copied to clipboard

wip - [7.0][IMP] account_financial_report_webkit general_ledger reduce sql …

Open tfossoulw opened this issue 6 years ago • 8 comments

…query nbr

This redure drastically the number of sql query to the database so optimise the report generation.

tfossoulw avatar May 24 '18 12:05 tfossoulw

This is changing some methods signature, it's a very old version and Travis is red, so I don't think this is going to be merged easily, as for sure no reviewers will arise. I think you should better focus on migrate your customers to newer versions.

Any way, I let it opened to see a bit.

@hbrunn what do you think about this?

pedrobaeza avatar May 24 '18 13:05 pedrobaeza

I don't think we should demotivate people working on old versions. But as you I'd be very cautious to merge this, because we don't have proper tests for 7.0. And I myself won't review it because I'm done with 7.0 indeed.

hbrunn avatar May 24 '18 13:05 hbrunn

It's not to demotivate, but being realistic. This is changing API signatures, and other modules depending on this one (OCA or not), might break after this change. The risk can be taken over a version well maintained (v10, v11, even v8...), but not v7

pedrobaeza avatar May 24 '18 13:05 pedrobaeza

I could also keep the method signature intact but adapt the method to allow 1 or more "account_id". Then other modules depending on this one shouldn't be impacted by this commit. ?

tfossoulw avatar May 24 '18 15:05 tfossoulw

There is the same problem, as if you pass several accounts, other modules won't process this multi-record properly.

pedrobaeza avatar May 24 '18 15:05 pedrobaeza

No, you allow to pass either one or several account. So other modules could continue to work with it.

tfossoulw avatar May 25 '18 07:05 tfossoulw

As v8 and v7 versions for impacted module are pretty similar another approach could be create the PR for v8 that have many more test and when accepted for v8 make a backport for v7.

My 2cents

agb80 avatar May 28 '18 16:05 agb80

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

github-actions[bot] avatar Aug 14 '22 12:08 github-actions[bot]