-
Notifications
You must be signed in to change notification settings - Fork 474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prints benchmark results in a neat table and attempts to run every benchmark #1464
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1464 +/- ##
==========================================
- Coverage 85.79% 85.75% -0.04%
==========================================
Files 646 647 +1
Lines 72051 72300 +249
==========================================
+ Hits 61813 61998 +185
- Misses 10238 10302 +64 ☔ View full report in Codecov by Sentry. |
Is the code ok to get merged ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't reviewed closely - leaving it up to @syl20bnr but I had small suggestion. It'd be convenient if we make the table Markdown compatible so we can copy the table output into any MD renderer (Discord, Github, other docs).
Intersting idea, that can be done. 🤝 |
|
Below is the rendering in this github's comment box ==========Benchmark Results==========
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome ! Nothing to say on the actual implementation. I have just minor change requests on the form. For the intermediary file we should create it in the cache directory explicitly.
Hi @syl20bnr, I've made changes according to the suggestions you've provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you !
@akhildevelops I forget to tell you in case you did not know it that you should not use the main branch of your fork when you submit a pull request and instead you should create a dedicate branch for each PR. This way you can more easily work on multiple PRs at once and keep the main branch up to date with the upstream repository. |
Sure Thanks 🤝 |
Pull Request Template
Checklist
run-checks all
script has been executed.Related Issues/PRs
Fixes #1440
Changes
Benchmark results will be displayed as below based on benchmarks X backends
Benchmark process will continue to run even if there's a failure in any one of the combination of benchmarks and backends
Testing
There's a test case included and ran this feature across all the available benchmarks and backends.