account-financial-reporting
account-financial-reporting copied to clipboard
wip - [7.0][IMP] account_financial_report_webkit general_ledger reduce sql …
…query nbr
This redure drastically the number of sql query to the database so optimise the report generation.
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?
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.
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
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. ?
There is the same problem, as if you pass several accounts, other modules won't process this multi-record properly.
No, you allow to pass either one or several account. So other modules could continue to work with it.
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
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.