FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

style: add pre-commit by Tieqiong · Pull Request #41 · diffpy/libdiffpy · GitHub

Repository navigation

style: add pre-commit - #41

Merged
sbillinge merged 2 commits into
diffpy:windowsfrom
Tieqiong:linting
Jun 26, 2025
Merged

sbillinge merged 2 commits into
diffpy:windowsfrom
Tieqiong:linting

Conversation

Copy link
Copy Markdown
Contributor
  1. Add pre-commit hooks and configuration files:
  • black, sort, flake8, codespell: standard skpkg configuration
  • clang-format: Google style clang formatter

We should discuss on how we setup codespell here, as in regular skpkg packages we refer to ignore_words and ignore_lines in pyproject.toml, which doesn't exist here. I'm able to use the ignore_word file directly into pre-commit.yaml by making it a shell command (which force the comment symbol from ;; to # ), but I'm not sure how to add ignore_lines.

We might also want to add an extra cpplint check after some maintenance is done on the code. I already added it in .pre-commit-config.yaml but commented out for now.

  1. Remove travis ci and local conda recipe: I think we don't need them anymore.

@sbillinge please check, thanks!

Copy link
Copy Markdown
Contributor

We should discuss on how we setup codespell here, as in regular skpkg packages we refer to ignore_words and ignore_lines in pyproject.toml, which doesn't exist here. I'm able to use the ignore_word file directly into pre-commit.yaml by making it a shell command (which force the comment symbol from ;; to # ), but I'm not sure how to add ignore_lines.

I would add a pyproject.toml. it only needs to have the [tool.black] and [tool.codespell] blocls in it. Nothing about building the package. Will this break setuptools for everything else if we do that?

Copy link
Copy Markdown
Contributor

cpplint would be good but let's get everything working first.

Copy link
Copy Markdown
Contributor

remove travis and conda recipe": correct

Copy link
Copy Markdown
Contributor

is there a reason why the tests are not funning on the PR? Do tests pass?

Copy link
Copy Markdown
Contributor Author

@sbillinge Thanks for the comments. This cpp library doesn't use setuptool, and I think adding a pyproject.toml is fine.

We never got the original tests to work, and I believe we discussed this before and decided to rewrite all tests using modernized cpp testing suits. All the tests on this for now depend on diffpy.srreal.

sbillinge merged commit 1785ad7 into diffpy:windows Jun 26, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL