-
Notifications
You must be signed in to change notification settings - Fork 286
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
cleanup: Fix a few more clang-tidy warnings. #2405
Conversation
ad1930c
to
608c881
Compare
36c3a24
to
722098a
Compare
Which warning? Variable assigned but never used? |
Speaking of warnings, is there a way to make a custom token warning, so that we would get warned if someone uses |
We have that, yes. In one of the builds we get an error for it. |
"Variable not initialised" |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #2405 +/- ##
==========================================
- Coverage 74.57% 74.49% -0.09%
==========================================
Files 87 87
Lines 26194 26194
==========================================
- Hits 19533 19512 -21
- Misses 6661 6682 +21 ☔ View full report in Codecov by Sentry. |
I've changed it back. This change conflicts too much with other warnings that are more useful. |
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.
Reviewed 3 of 87 files at r1, 84 of 84 files at r2, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @nurupo)
@nurupo PTAL. I've addressed your comments, I think (and also reverted most of it). |
Ah, sorry for the wait, |
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.
LGTM
Actually that's false, they do stand out, was just looking at the wrong gmail label when writing this. |
This change is