Skip to content
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

CI: add pre-commit.ci to apply clang-format and clang-tidy fixes #4225

Merged
merged 22 commits into from
Jun 13, 2024

Conversation

caic99
Copy link
Member

@caic99 caic99 commented May 24, 2024

Linked Issue

Fix #3915

What's changed?

This PR add the CI test static checks using clang-format and clang-tidy. It uses pre-commit.ci to orchestrate the checks and submit patches. On new commits pushing, it will try to fix the code smells in changed files.

For example, a developer modified the code without caring indentations in c17732b. The commit triggered workflow runs, and pre-commit.ci submited a patch for it in 039d5a0.

This PR also updates the config files used by those tools. Please submit an issue if the suggested changes by those rules does not fit the circumstance well.

@mohanchen mohanchen added the Compile & CICD & Docs Issues related to compiling ABACUS label May 25, 2024
caic99 and others added 12 commits May 27, 2024 10:47
ABACUS uses some functions with a long function name and a long parameter list.

Current setup will generates ugly parameter list with each parameters indented by many spaces. Plus, it is unable to use a customized layout for e.g. putting x, y, and z value in the same line.

After discussion, we agree to disable those settings, leaving developers free to chose their own flavor.
@caic99 caic99 marked this pull request as ready for review May 27, 2024 08:13
Copy link
Collaborator

@dyzheng dyzheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I want to know how to make the decision to change clang-format in the future?

@caic99
Copy link
Member Author

caic99 commented May 29, 2024

LGTM, but I want to know how to make the decision to change clang-format in the future?

@dyzheng As I've mentioned, one can change the check rules if current rules mismatch their requirements. It is possible to polish the rules iteratively.

@mohanchen
Copy link
Collaborator

Great job! I need to check every rule in the clang-format and clang-tidy before merging this PR.

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@mohanchen mohanchen merged commit 10c5281 into deepmodeling:develop Jun 13, 2024
13 checks passed
@caic99 caic99 deleted the precommit branch June 13, 2024 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compile & CICD & Docs Issues related to compiling ABACUS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-format and clang-tidy is not employed
4 participants