-
Notifications
You must be signed in to change notification settings - Fork 986
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
add clang-tidy to GLCI #6580
base: master
Are you sure you want to change the base?
add clang-tidy to GLCI #6580
Conversation
.gitlab-ci.yml
Outdated
@@ -198,6 +198,7 @@ test-lin-dev-clang-cran: | |||
- echo 'CFLAGS=-g -O2 -fno-common -Wall -Wvla -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' > ~/.R/Makevars | |||
- echo 'CXXFLAGS=-g -O2 -fno-common -Wall -Wvla -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' >> ~/.R/Makevars | |||
- *install-deps | |||
- clang-tidy -extra-arg=-I/usr/local/lib/R/include -checks='readability-inconsistent-declaration-parameter' src/*.c -- -std=c11 |
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.
what will this look like for manually specifying, say, dozens of checks? Will it just be
clang-tidy -extra-arg=... \
-checks='...' \
src/*.c
Is it possible to keep this as a list in a file (say in .ci)?
(doesn't need to be done in this PR since we only have one check thus far, just trying to think ahead before it gets much harder to refactor)
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.
Yes. We can also use globbing for checks e.g. -check='readability-*'
or multiple checks with -checks='bugprone-*,readability-*'
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6580 +/- ##
==========================================
- Coverage 98.62% 98.60% -0.02%
==========================================
Files 79 79
Lines 14450 14516 +66
==========================================
+ Hits 14251 14314 +63
- Misses 199 202 +3 ☔ View full report in Codecov by Sentry. |
@@ -198,6 +198,7 @@ test-lin-dev-clang-cran: | |||
- echo 'CFLAGS=-g -O2 -fno-common -Wall -Wvla -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' > ~/.R/Makevars | |||
- echo 'CXXFLAGS=-g -O2 -fno-common -Wall -Wvla -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' >> ~/.R/Makevars | |||
- *install-deps | |||
- clang-tidy -extra-arg=-I/usr/local/lib/R/include -checks='readability-inconsistent-declaration-parameter' src/*.c -- -std=c99 |
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.
Comments describing this job, here in the file, could mention about this check as well. Also .ci/README.MD could be updated for that.
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.
Updating the .ci/README.md and adding some best practices like developing CI changes on Gitlab and then fetching them here is on my long standing to-do list
Closes #2781
Right now I would only start with the
readability-inconsistent-declaration-parameter
check and move forward with adding additional checks when needed.edit: switched to C99 instead of C11 since C11 gave us a huge amount of false-positives
CI Results