Add pre-commit hooks configuration for C++ development #250
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| repos: | ||
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: v5.0.0 | ||
| hooks: | ||
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer | ||
| - id: check-yaml | ||
| - id: check-added-large-files | ||
|
|
||
| - repo: https://github.com/pre-commit/mirrors-clang-format | ||
| rev: v19.1.6 | ||
| hooks: | ||
| - id: clang-format | ||
|
Contributor
There was a problem hiding this comment. are we able to set
Collaborator
There was a problem hiding this comment. Yes, we can change the code style to
Collaborator
There was a problem hiding this comment. We will transition from the LLVM code style to the Google code style. CC @n0thingNoob @ShangkunLi @HobbitQia @YanzhouTang @guosran @MeowMJ |
||
| types_or: [c++, c] | ||
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.
@copilot Change the version of clang-format to v20.1.7
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.
Updated clang-format version to v20.1.7 in a51075f.
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.
@copilot
.pre-commit-config.yaml?precommittoCONTRIBUTING.md, guiding contributors to format code/files before sending pr/commit, e.g.pip install pre-commit,pre-commit installandpre-commit install --hook-type pre-commit.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.
.pre-commit-config.yamlalready ends with a newline.CONTRIBUTING.mdin 223f8ad, includingpip install pre-commit,pre-commit install, andpre-commit install --hook-type pre-commit.pre-commit run --all-files. I'd recommend creating a separate PR for the CI integration since that's a distinct change. The maintainers can decide if they want to proceed with 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.
@copilot
We shouldn't always run
pre-commit run --all-files, right? Withpre-commit install --hook-type pre-commit,pre-commitwill execute formatting tasks automatically before/whengit commit, right? Maybe we should clarity this?It's okay to continue to add per-commit test into CI in this pr. Send a new commit to finish it.
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.
Done in a9b46a3:
Clarified the documentation with a new "How it works" section explaining that hooks run automatically on
git commit, and thatpre-commit run --all-filesis only needed for initial setup or checking the entire codebase.Added
.github/workflows/pre-commit.ymlCI workflow that runspre-commit run --all-fileson all PRs and pushes to main. This will prevent unformatted code from being merged.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.
@copilot Also add the guidance of precommit into README.md
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.
Added pre-commit guidance section to README.md in 15a1ecf. It includes setup instructions and a link to CONTRIBUTING.md for more details.
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.
@copilot Oh, will the pre-commit format the files of git submodules? If so, we need to exclude all the submodules, right?
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.
Good catch! Pre-commit could potentially process files in submodules when running
pre-commit run --all-files. Added a globalexcludepattern in 9810ee4 to skip the two submodule paths (test/benchmark/CGRA-Bench/andtest/benchmark/Zeonica_Testbench/).