-
Notifications
You must be signed in to change notification settings - Fork 19
17 Timing framework #1179
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
17 Timing framework #1179
Conversation
964bf5d to
3ca277c
Compare
implement TablePrinter and ListPrinter, move TimerRegistration to avoid circular dependencies, change default printing behaviour of TimerRegistrar
…nto 17-time-measurement
A lot of added documentation.
…nto 17-time-measurement
add more tests
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1179 +/- ##
==========================================
- Coverage 97.21% 97.20% -0.02%
==========================================
Files 156 167 +11
Lines 14674 14915 +241
==========================================
+ Hits 14266 14498 +232
- Misses 408 417 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
HenrZu
left a comment
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.
The code is incredibly good and well structured. I only have one comment in the BasicTimer. Otherwise, i have read through the whole doc and found a few typos. The doc helped a lot to understand the structure. Great work :)
A readme in the time dir would be helpful. Tests and Readthedocs documentation is also missing (as you said before :) )
HenrZu
left a comment
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.
Looks great :) Only short comments/suggestions.
I'm still unsure about the namespace, which is better :D But we should keep it consistent. So either we name the directory in timing, or the namespace in mio::time.
|
i just linked the issue for the bump of the c++ version. Could you also update the c++ version here:
|
Co-authored-by: Henrik Zunker <69154294+HenrZu@users.noreply.github.com>
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
If need be, add additional information and what the reviewer should look out for in particular:
Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)
Closes #17