-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: use clang-tidy lint readability-identifier-naming #13642
Open
Swiftb0y
wants to merge
12
commits into
mixxxdj:main
Choose a base branch
from
Swiftb0y:feat/clang-tidy-naming
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3caf06a
feat: use clang-tidy lint readability-identifier-naming
Swiftb0y e9587bb
generator compile_commands.json unconditionally (for clang-tidy)
Swiftb0y c3da7b1
add clang-tidy pre-commit hook
Swiftb0y dcf39f2
remove cmake clang-tidy code
Swiftb0y 7fc3362
remove clang-tidy CI as the diff is now checked pre-commit
Swiftb0y 4abc2b2
remove outdated workaround
Swiftb0y ce7879d
fixup! add clang-tidy pre-commit hook
Swiftb0y 2369daa
fixup! add clang-tidy pre-commit hook
Swiftb0y 8cf4b52
fixup! fixup! add clang-tidy pre-commit hook
Swiftb0y 79d9a99
fixup! fixup! fixup! add clang-tidy pre-commit hook
Swiftb0y 3ea3dcc
fixup! remove cmake clang-tidy code
Swiftb0y 64a087a
fixup! add clang-tidy pre-commit hook
Swiftb0y File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 file contains 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 file contains 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 file contains 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 file contains 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 file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| #!/usr/bin/env python3 | ||
| # -*- coding: utf-8 -*- | ||
| import argparse | ||
| import logging | ||
| import os | ||
| import subprocess | ||
| import sys | ||
| import typing | ||
| import json | ||
|
|
||
| import githelper | ||
|
|
||
|
|
||
| # Function to transform FileLines to desired JSON format | ||
| def file_lines_to_json( | ||
| file_lines_list: typing.List[githelper.FileLines], | ||
| ) -> str: | ||
| json_list = [] | ||
| for file in file_lines_list: | ||
| # Transform each file to the desired format | ||
| file_obj = {"name": file.filename} | ||
| # Only include "lines" key if lines exist | ||
| if file.lines: | ||
| file_obj["lines"] = [list(line) for line in file.lines] | ||
| json_list.append(file_obj) | ||
|
|
||
| return json.dumps(json_list, separators=(",", ":")) | ||
|
|
||
|
|
||
| def find_compile_commands(rootdir): | ||
| for root, dirs, files in os.walk(rootdir): | ||
| if "compile_commands.json" in files: | ||
| return os.path.join(root, "compile_commands.json") | ||
| return None | ||
|
|
||
|
|
||
| def run_clang_tidy_on_lines(rootdir, lines, compile_commands): | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
| line_filter = file_lines_to_json(lines) | ||
| assert line_filter | ||
|
|
||
| filenames = [os.path.join(rootdir, file.filename) for file in lines] | ||
|
|
||
| cmd = [ | ||
| "clang-tidy", | ||
| "--format-style=file", | ||
| "--fix-notes", | ||
| "--fix-errors", | ||
| "--warnings-as-errors=*", | ||
| "-p=" + compile_commands, | ||
| "--line-filter=" + line_filter, | ||
| *filenames, | ||
| ] | ||
|
|
||
| logger.info(f"running on {line_filter}") | ||
| proc = subprocess.run(cmd, capture_output=True, text=True) | ||
| if proc.returncode != 0: | ||
| logger.error(proc.stdout) | ||
| sys.exit(1) | ||
|
|
||
|
|
||
| def main(argv: typing.Optional[typing.List[str]] = None) -> int: | ||
| logging.basicConfig( | ||
| format="[%(levelname)s] %(message)s", level=logging.INFO | ||
| ) | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| parser = argparse.ArgumentParser() | ||
| parser.add_argument("--from-ref", help="use changes changes since commit") | ||
| parser.add_argument("--to-ref", help="use changes until commit") | ||
| parser.add_argument("files", nargs="*", help="only check these files") | ||
| args = parser.parse_args(argv) | ||
|
|
||
| if not args.from_ref: | ||
| args.from_ref = os.getenv("PRE_COMMIT_FROM_REF") or os.getenv( | ||
| "PRE_COMMIT_SOURCE" | ||
| ) | ||
|
|
||
| if not args.to_ref: | ||
| args.to_ref = os.getenv("PRE_COMMIT_TO_REF") or os.getenv( | ||
| "PRE_COMMIT_ORIGIN" | ||
| ) | ||
|
|
||
| # Filter filenames | ||
| rootdir = githelper.get_toplevel_path() | ||
|
|
||
| logger.debug("Looking for compile_commands.json...") | ||
| compile_commands = find_compile_commands(rootdir) | ||
|
|
||
| if not compile_commands: | ||
| logger.error("could not find compile_commands.json\n") | ||
| sys.exit(1) | ||
|
|
||
| logger.debug("getting changed lines") | ||
| changed_lines = githelper.get_changed_lines_grouped( | ||
| from_ref=args.from_ref, | ||
| to_ref=args.to_ref, | ||
| filter_lines=lambda line: line.added, | ||
| include_files=args.files, | ||
| ) | ||
| logger.debug("running clang-tidy") | ||
| run_clang_tidy_on_lines(rootdir, list(changed_lines), compile_commands) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) |
Oops, something went wrong.
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.
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.
This is a quite costly operation IMO - how do you feel about updating the Docker image instead? Note that would could also make the container in #14018 pushed in GHCR and reuse it in our CI? It may actually be quite good as we could use it
build(Ubuntu) as well and a save a significant amount of bandwidth/resource consumptionThere 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.
yeah, I agree. this was just a quick and dirty PoC that has stalled now (because of issues like these). Updating the docker would def. be preferred.
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.
@mixxxdj/developers any objection in me extending the scope of #14018 and publish devcontainer in Mixxx GHCR repo?