Make results' limits configurable (fixes #347)
Context
There were several places in code with hardcoded limits for items rendered on charts. It was really inconvenient for the end-user who had to rebuild the entire project to make limits configurable.
This fix moves the actual configuration into an Aggregator interface which is implemented by key plugins. Moreover, a user can now control the limits via ALLURE_RESULTS_LIMIT env variable that could be set before the report's generation. Note that a newly added method was intentionally made static as some of the APIs that adjust the limits are also static.
Fixes #347
Checklist
- [x] Sign Allure CLA
- [ ] Provide unit tests
Hello, Just checking to see if there is a possibility of this PR to get merged soon and what it would take to get this merged :)
Thank you.
@indradeepchowdhury as far as I remember, @baev had some concerns in terms of implementation. Need to double-check with him.
@indradeepchowdhury as far as I remember, @baev had some concerns in terms of implementation. Need to double-check with him.
Thanks for the update @sskorol. @baev would it be possible to expedite this feature. This would be a super cool feature to have.
Hi @sskorol. Thanks for jour work on providing such feature. I've got one question - do you still work on this or has it been parked?
Hi @pbandura, I don't. When I initially pushed it, it was a fully working feature. I talked to maintainers directly. And they promised to check. Seems like they didn't have time. Now I see there's a conflict. I can resolve it. And then you will be able to build a new version with this feature from sources if it's important for you.
Hi @sskorol I think this feature would be pretty cool and I am looking forward to using it. This feature will allow us to align the history with our Sprint duration or even let us increase the history limit to a month for proper tracking 🙂
@indradeepchowdhury @pbandura force-pushed changes with conflicts' resolution. Now you can build the report on your own with this feature.
@sskorol @indradeepchowdhury @baev Why didn't you guys apply this?
I don't like the current implementation because:
- Plugin API changes -- new method introduced in Aggregator interface may create some issues for other users
- The env variable processing is unsafe -- You can set negative or non-number env variable and crash your report generation
- There are no tests provided
Besides that, our team is heavily focused on Allure 3 development right now, so this change isn't really in priority right now