-
Notifications
You must be signed in to change notification settings - Fork 65
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
Minor refactorings in the query planning for property paths #1805
Minor refactorings in the query planning for property paths #1805
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1805 +/- ##
==========================================
+ Coverage 90.02% 90.04% +0.02%
==========================================
Files 396 396
Lines 37974 37967 -7
Branches 4262 4261 -1
==========================================
+ Hits 34185 34187 +2
+ Misses 2493 2487 -6
+ Partials 1296 1293 -3 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Johannes Kalmbach <[email protected]>
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 very much. I have fixed two additional trivial sonarcloud issues and removed some [[nodiscard]]
statements which clutter the code
I will merge this after the checks have run through again
PropertyPath::Operation op) { | ||
PropertyPath p(std::move(op)); | ||
p._children = std::move(children); | ||
const Operation op) { |
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.
const Operation op) { | |
const Operation& op) { |
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.
No, trivially copyable small types can be passed by value, the pointer is an overhead:)
The const is for the usage within the function:)
Conformance check passed ✅No test result changes. |
|
PropertyPath
related code
Those include fixing smaller sonarcloud issues and addressing non-functional
TODO
statements in the code.