Skip to content

add and run pre-commit to format codes#2279

Merged
wanghan-iapcm merged 5 commits into
deepmodeling:develfrom
njzjz:pre-commit
Feb 3, 2023
Merged

add and run pre-commit to format codes#2279
wanghan-iapcm merged 5 commits into
deepmodeling:develfrom
njzjz:pre-commit

Conversation

@njzjz

@njzjz njzjz commented Jan 25, 2023

Copy link
Copy Markdown
Member

Including Black for Python, Clang format for C++, and others for Shell, CMake, JSON, and CSS.

@njzjz njzjz marked this pull request as draft January 25, 2023 19:40
@njzjz njzjz requested a review from wanghan-iapcm January 25, 2023 20:07
@njzjz njzjz marked this pull request as ready for review January 25, 2023 20:07
@njzjz njzjz changed the title add and run pre-commit add and run pre-commit to format codes Jan 26, 2023
@njzjz

njzjz commented Jan 26, 2023

Copy link
Copy Markdown
Member Author

@wanghan-iapcm It should be discussed what variant of the Clang format we prefer. Clang documentation provides a lot of styles (default is llvm).

Comment thread source/lib/src/fmt_nlist.cc Outdated
@wanghan-iapcm

Copy link
Copy Markdown
Collaborator

@wanghan-iapcm It should be discussed what variant of the Clang format we prefer. Clang documentation provides a lot of styles (default is llvm).

Let me check

@njzjz

njzjz commented Jan 27, 2023

Copy link
Copy Markdown
Member Author
  • Done: json.hpp should be skipped.

@codecov-commenter

codecov-commenter commented Jan 27, 2023

Copy link
Copy Markdown

Codecov Report

Base: 75.27% // Head: 74.58% // Decreases project coverage by -0.69% ⚠️

Coverage data is based on head (7f14cd1) compared to base (37c38dc).
Patch coverage: 81.93% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #2279      +/-   ##
==========================================
- Coverage   75.27%   74.58%   -0.69%     
==========================================
  Files         217      217              
  Lines       21109    21494     +385     
  Branches     1479     1563      +84     
==========================================
+ Hits        15889    16031     +142     
- Misses       4281     4447     +166     
- Partials      939     1016      +77     
Impacted Files Coverage Δ
deepmd/__about__.py 0.00% <0.00%> (ø)
deepmd/lmp.py 0.00% <0.00%> (ø)
deepmd/nvnmd/utils/config.py 91.30% <ø> (-0.05%) ⬇️
deepmd/nvnmd/utils/encode.py 83.43% <ø> (ø)
deepmd/nvnmd/utils/fio.py 72.59% <ø> (ø)
deepmd/nvnmd/utils/network.py 59.00% <ø> (-0.81%) ⬇️
deepmd/nvnmd/utils/op.py 64.28% <ø> (ø)
deepmd/nvnmd/utils/weight.py 94.59% <ø> (ø)
deepmd/op/__init__.py 100.00% <ø> (ø)
deepmd/train/run_options.py 85.84% <ø> (-2.66%) ⬇️
... and 251 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wanghan-iapcm

Copy link
Copy Markdown
Collaborator

@njzjz Shall we wait PR #2253 and #2254?

@njzjz

njzjz commented Jan 27, 2023

Copy link
Copy Markdown
Member Author

@njzjz Shall we wait PR #2253 and #2254?

Not required. There was a similar PR in scikit-learn/scikit-learn#18948, and scikit-learn/scikit-learn#20301 provides an automatic way to fix conflicts.

@njzjz njzjz marked this pull request as draft January 28, 2023 03:19
wanghan-iapcm pushed a commit that referenced this pull request Jan 28, 2023
Prepare #2279.

- Fix some C++ headers which didn't include the std library.
- Add Clang-format config file.
- Remove the old lint Python action.
@njzjz

njzjz commented Feb 3, 2023

Copy link
Copy Markdown
Member Author

pre-commit.ci autofix

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz njzjz requested a review from wanghan-iapcm February 3, 2023 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants