-
Notifications
You must be signed in to change notification settings - Fork 339
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
Added benchmark score for PrivalFromString (#378) #379
Added benchmark score for PrivalFromString (#378) #379
Conversation
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.
Thanks very much for putting this together! I've included a few comments for you to address, after which I will run the CI tests.
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## latest #379 +/- ##
==========================================
+ Coverage 91.69% 91.78% +0.09%
==========================================
Files 44 44
Lines 3743 3772 +29
Branches 489 497 +8
==========================================
+ Hits 3432 3462 +30
Misses 216 216
+ Partials 95 94 -1 ☔ View full report in Codecov by Sentry. |
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.
Everything looks good now! The final thing for you to correct is simply add an include for <string>
to priority.cpp
(due to the usage of std::string
), which should satisfy the include check in the Static Analysis workflow.
This adds benchmark testing for PrivalFromString. This is intended to compare benchmarking score for reference in the future if further optimisations/changes done in the corresponding function.
Make sure that your pull request follows the guidelines specified in the
Guidelines for Contributing.