Conversation
dc4b510 to
f5d5efe
Compare
|
pre-commit fails due to clang-format. @lan496, previously clang-format didn't check following lines? |
|
@atztogo It seems. In my local environment (clang-format==16.0.4), formatted to |
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## develop #278 +/- ##
===========================================
- Coverage 85.92% 85.92% -0.01%
===========================================
Files 23 23
Lines 6082 6081 -1
===========================================
- Hits 5226 5225 -1
Misses 856 856
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
@LecrisUT It seems the versions of clang-format in my environment and CI are diffenrent. The CI's clang-format will be handled at https://github.com/Takishima/cmake-pre-commit-hooks. Do you know how to know and specify the clang-format version in CI? |
Oh yeah I had this issue before. We can use pre-commit-ci to get newer versions and/or use a container with pre-installed modern clang-format. Also have to check if we can we can enforce a clang-format minimum version. |
| add_compile_options(/W4) | ||
| else() | ||
| add_compile_options(-Wall -Wextra -Wpedantic) | ||
| endif() |
There was a problem hiding this comment.
How many warnings are there to address. Is it eventually feasible to convert the warnings to errors for the CI?
Oh, I didn't know there is a PyPI version of clang-format as well (reference). Maybe can specify the version there. |
f5d5efe to
49516a0
Compare
|
At spglib/.pre-commit-config.yaml Lines 9 to 12 in b2380a3 Try: - repo: https://github.com/Takishima/cmake-pre-commit-hooks
rev: v1.5.3
hooks:
- id: clang-format
additional_dependencies: [ clang-format >= 16 ] |
|
OK, it seems the CI uses clang-format>=16 now! I'll clean up the commits. |
ffb100c to
7373cc0
Compare
|
Add to todo:
|
Note: The input `max_size` is removed from `get_symmetry_with_site_tensors` because this input is not used in the function.
7373cc0 to
0733349
Compare
|
Ok, the final issue is about my last comment
Can you make an issue about it if we are not addressing these issues in this PR? |
|
Sure! I've opened the issue #289. |
This PR adds useful compiler warning flags depending on the compilers.
Ref: https://cmake.org/cmake/help/latest/command/add_compile_options.html#example