-
Notifications
You must be signed in to change notification settings - Fork 147
Add github action and make targets for clang-tidy and iwyu. #229
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a "linter" that checks code for errors, taken from PR librsync#222 by riszotto.
It seems the ubuntu iwyu package has a bug where it's missing a dependency containing the headers it needs.
This is to try and figure out why iwyu is still not finding standard headers.
This is the missing dependency needed by iwyu.
The clang-analyzer-valist.Uninitialized check has a bug that gives false-positives. See https://bugs.llvm.org/show_bug.cgi?id=41311.
Fixed stats.c and sumset_test.c for minor clang warnings about unused values.
Always use FILENAME_H as the include guard name for filename.h. Also make commens on include guards consistent.
This was mostly done by piping iwyu_tool output to `fix_include --noblank_lines` with the exception of hashtable.h and its unittest which required manual tweaking, plus a few manual tweaks in some places. For fileutil.c it was excluded and only manually tweaked a tiny bit. It has conditional includes based on config.h settings which seems to confuse iwyu_tool and fix_includes to the point where it recommended removing several of them which wouldn't compile any more.
This uses iwyu_tool output fed to `fix_include --noblank_lines` and excludes fileutil.c which gets messed up by this.
Restructure the document into headings that better represent the development processes. Update and expand the entries in each section to include new make targets and github actions.
Member
Author
|
I'm happy with this now, and I think I've dotted all the i's and crossed all the t's. I'm going to merge this now. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds clang-tidy and iwyu make targets for linting code, and adds a lint.yml githup action to run them.
Fixes #215 and replaces #222. This takes ideas from rizsotto's #222 pull request and changes them in the following ways;
analyze-buildchecks which research indicates are a subset ofclang-tidy.CMAKE_EXPORT_COMPILE_COMMANDS ONinCMakeLists.txtso it doesn't need to be set at the cmdline.clang-tidyandiwyumake targets, so that they can easily be executed at the cmdline too.iwyu-fixmake target for automatically fixing headers usingfix_include. This skipsfileutil.cwhich confuses these tools by conditionally including all the available file related headers to try and find working functions for different platforms.clang-tidy. There was actually not many.iwyuexcept forfileutil.c. This included making the include guards consistently named, adding include guards to all the headers that didn't have them, and fixing all the includes.CONTRIBUTING.mdto add a Requirements section listing dependencies and support tools to install, and how to use the new make targets.NEWS.mdwith changes added so far.