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
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ BasedOnStyle: Microsoft

AlwaysBreakTemplateDeclarations: Yes

AllowAllArgumentsOnNextLine: false
AllowAllParametersOfDeclarationOnNextLine: false
BinPackArguments: false
BinPackParameters: false
AllowAllArgumentsOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
BinPackArguments: true
BinPackParameters: true

BreakBeforeBinaryOperators: All
BreakBeforeTernaryOperators: true
Expand Down
18 changes: 12 additions & 6 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
Checks: '
-*,
bugprone-*,
clang-analyzer-*,
clang-diagnostic-*,
Expand All @@ -14,22 +13,29 @@ Checks: '
misc-*,
modernize-deprecated-headers,
modernize-redundant-void-arg,
modernize-use-nullptr,
modernize-use-bool-literals,
modernize-use-auto,
modernize-use-std-numbers,
modernize-pass-by-value,
modernize-shrink-to-fit,
modernize-use-bool-literals,
modernize-use-nullptr,
modernize-use-starts-ends-with,
mpi-*,
performance-*,
-performance-avoid-endl,
readability-*,
-readability-else-after-return,
-readability-magic-numbers,
-readability-isolate-declaration,
-readability-braces-around-statements,
-readability-implicit-bool-conversion,
-google-readability-braces-around-statements,
-misc-unused-parameters,
-modernize-pass-by-value,
-modernize-use-auto,
-google-readability-casting,
-readability-make-member-function-const,
-cppcoreguidelines-avoid-non-const-global-variables,
-readability-function-cognitive-complexity,
-google-build-using-namespace,
-bugprone-narrowing-conversions,
'
FormatStyle: file
...
21 changes: 16 additions & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,30 @@ jobs:
uses: actions/checkout@v4
with:
submodules: recursive
fetch-depth: 0

- name: Install Ccache
- name: Install CI tools
run: |
sudo apt-get update
sudo apt-get install -y ccache
sudo apt-get install -y ccache clang-format clang-tidy ca-certificates

- name: Build
- name: Configure
run: |
cmake -B build -DBUILD_TESTING=ON -DENABLE_DEEPKS=ON -DENABLE_LIBXC=ON -DENABLE_LIBRI=ON -DENABLE_PAW=ON -DENABLE_GOOGLEBENCH=ON -DENABLE_RAPIDJSON=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=1

- uses: actions/setup-python@v3
- uses: pre-commit/action@v3.0.1
with:
extra_args:
--from-ref ${{ github.event.pull_request.base.sha }}
--to-ref ${{ github.event.pull_request.head.sha }}
continue-on-error: true
- uses: pre-commit-ci/lite-action@v1.0.2

cmake -B build -DBUILD_TESTING=ON -DENABLE_DEEPKS=ON -DENABLE_LIBXC=ON -DENABLE_LIBRI=ON -DENABLE_PAW=ON -DENABLE_GOOGLEBENCH=ON -DENABLE_RAPIDJSON=ON
- name: Build
run: |
cmake --build build -j8
cmake --install build

- name: Test
env:
GTEST_COLOR: 'yes'
Expand Down
14 changes: 14 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
fail_fast: false
repos:
- repo: https://github.com/pocc/pre-commit-hooks
rev: v1.3.5
hooks:
- id: clang-format
args: [-i]
- id: clang-tidy
args: [-p=build, --fix-errors]
caic99 marked this conversation as resolved.
Show resolved Hide resolved
# - id: oclint
# - id: uncrustify
# - id: cppcheck
# - id: cpplint
# - id: include-what-you-use
55 changes: 37 additions & 18 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,23 @@ We use `clang-format` as our code formatter. The `.clang-format` file in root di
For Visual Studio Code developers, the [official extension of C/C++](https://marketplace.visualstudio.com/items?itemName=ms-vscode.cpptools) provided by Microsoft can help you format your codes following the rules. With this extension installed, format your code with `shift+command/alt+f`.
Configure your VS Code settings as `"C_Cpp.clang_format_style": "file"` (you can look up this option by pasting it into the search box of VS Code settings page), and all this stuff will take into effect. You may also set `"editor.formatOnSave": true` to avoid formatting files everytime manually.

We use <https://pre-commit.ci/> to format the code.
It is performed after pushing new commits to a PR. You might need to pull the changes before adding new commits.

To use pre-commit locally (**generally not required**):
Please install the pre-commit tool by running the following command:

```bash
pip install pre-commit
pip install clang-tidy clang-format # if you haven't installed them
```

Then, run the following command to install the pre-commit hooks:

```bash
pre-commit install
```

## Adding a unit test

We use [GoogleTest](https://github.com/google/googletest) as our test framework. Write your test under the corresponding module folder at `abacus-develop/tests`, then append the test to `tests/CMakeLists.txt`. If there are currently no unit tests provided for the module, do as follows. `module_base` provides a simple demonstration.
Expand Down Expand Up @@ -386,33 +403,35 @@ To run a subset of unit test, use `ctest -R <test-match-pattern>` to perform tes
git push origin my-fix-branch
```

6. In GitHub, send a pull request (PR) with `deepmodeling/abacus-develop:develop` as the base repository. It is required to document your PR following [our guidelines](#commit-message-guidelines).
6. In GitHub, send a pull request (PR) with `deepmodeling/abacus-develop:develop` as the base repository. It is **required** to document your PR following [our guidelines](#commit-message-guidelines).

7. After your pull request is merged, you can safely delete your branch and sync the changes from the main (upstream) repository:
7. If more changes are needed, you can add more commits to your branch and push them to GitHub. Your PR will be updated automatically.

- Delete the remote branch on GitHub either [through the GitHub web UI](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-branches-in-your-repository/deleting-and-restoring-branches-in-a-pull-request#deleting-a-branch-used-for-a-pull-request) or your local shell as follows:
8. After your pull request is merged, you can safely delete your branch and sync the changes from the main (upstream) repository:

```shell
git push origin --delete my-fix-branch
```
- Delete the remote branch on GitHub either [through the GitHub web UI](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-branches-in-your-repository/deleting-and-restoring-branches-in-a-pull-request#deleting-a-branch-used-for-a-pull-request) or your local shell as follows:

- Check out the master branch:
```shell
git push origin --delete my-fix-branch
```

```shell
git checkout develop -f
```
- Check out the master branch:

- Delete the local branch:
```shell
git checkout develop -f
```

```shell
git branch -D my-fix-branch
```
- Delete the local branch:

- Update your master with the latest upstream version:
```shell
git branch -D my-fix-branch
```

```shell
git pull --ff upstream develop
```
- Update your master with the latest upstream version:

```shell
git pull --ff upstream develop
```

## Commit message guidelines

Expand Down
20 changes: 8 additions & 12 deletions source/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Driver::Driver()

Driver::~Driver()
{
// Release the device memory within singleton object GlobalC::ppcell
// Release the device memory within singleton object GlobalC::ppcell
// before the main function exits.
GlobalC::ppcell.release_memory();
}
Expand All @@ -25,31 +25,30 @@ void Driver::init()
{
ModuleBase::TITLE("Driver", "init");

time_t time_start = std::time(NULL);
time_t time_start = std::time(nullptr);
ModuleBase::timer::start();

// (1) read the input parameters.
// INPUT should be initalized here and then pass to atomic world, mohan 2024-05-12
// INPUT should not be GlobalC, mohan 2024-05-12
this->reading();
Driver::reading();

// (2) welcome to the atomic world!
this->atomic_world();

// (3) output information
time_t time_finish = std::time(NULL);
time_t time_finish = std::time(nullptr);
Print_Info::print_time(time_start, time_finish);

// (4) close all of the running logs
INPUT.close_log();

// (5) output the json file
//Json::create_Json(&GlobalC::ucell.symm,GlobalC::ucell.atoms,&INPUT);
Json::create_Json(&GlobalC::ucell,&INPUT);
return;
// Json::create_Json(&GlobalC::ucell.symm,GlobalC::ucell.atoms,&INPUT);
Json::create_Json(&GlobalC::ucell, &INPUT);
}

void Driver::reading(void)
void Driver::reading()
{
ModuleBase::timer::tick("Driver", "reading");

Expand Down Expand Up @@ -83,10 +82,9 @@ void Driver::reading(void)
// ModuleBase::GlobalFunc::DONE(GlobalV::ofs_running,"READING CARDS");

ModuleBase::timer::tick("Driver", "reading");
return;
}

void Driver::atomic_world(void)
void Driver::atomic_world()
{
ModuleBase::TITLE("Driver", "atomic_world");
//--------------------------------------------------
Expand All @@ -101,6 +99,4 @@ void Driver::atomic_world(void)

ModuleBase::timer::finish(GlobalV::ofs_running);
ModuleBase::Memory::print_all(GlobalV::ofs_running);

return;
}
4 changes: 2 additions & 2 deletions source/driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ class Driver
* This function read the parameter in "INPUT", "STRU" etc,
* and split the MPI world into different groups.
*/
void reading();
static void reading();
dyzheng marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief An interface function.
* This function calls "this->driver_run()" to do calculation,
* and log the time and memory consumed during calculation.
* and log the time and memory consumed during calculation.
*/
void atomic_world();

Expand Down
Loading