Skip to content

pre-commit#173

Merged
ryanmrichard merged 8 commits into
masterfrom
pre-commit_update
Aug 11, 2025
Merged

pre-commit#173
ryanmrichard merged 8 commits into
masterfrom
pre-commit_update

Conversation

@jwaldrop107

Copy link
Copy Markdown
Member

Is this pull request associated with an issue(s)?
No.

Description
This PR adds the instructions for configuring pre-commit hooks using the configuration file found NWChemEx/pre-commit-config (and why the config lives in that repo). Most of the (rather large amount of) changes in this PR are a result of applying license headers and trimming whitespace courtesy of the pre-commit hooks. The main point of review is the new additions to the developer documentation found in docs/source/conventions/pre_commit.rst. Relatedly, the reviewers should take a look at the .pre-commit-config.yaml in NWChemEx/pre-commit-config.

@jwaldrop107 jwaldrop107 self-assigned this Aug 7, 2025

@zachcran zachcran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a few minor requested changes and comments. Overall, looks good!

Comment thread docs/source/conventions/pre_commit.rst
Comment thread docs/source/conventions/pre_commit.rst Outdated
Comment thread docs/source/conventions/pre_commit.rst Outdated
Comment thread docs/source/conventions/pre_commit.rst
Comment thread docs/source/conventions/pre_commit.rst Outdated
Comment thread .licenserc.yaml Outdated
@zachcran

zachcran commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

@jwaldrop107 How would you like me to provide feedback on the .pre-commit-config.yaml file if I have any?

jwaldrop107 and others added 2 commits August 7, 2025 16:55
Co-authored-by: Zachery Crandall <15938371+zachcran@users.noreply.github.com>
@jwaldrop107

jwaldrop107 commented Aug 7, 2025

Copy link
Copy Markdown
Member Author

@zachcran We can discuss it here, or you can start an issue in the pre-commit-config repo.

@zachcran

zachcran commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

I'll open an issue for discussion there since most/all of it probably won't affect this PR's contents.

@zachcran

zachcran commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

@jwaldrop107 I opened the following issue for .pre-commit-config.yaml discussions: NWChemEx/pre-commit-config#1.

@jwaldrop107 jwaldrop107 requested a review from zachcran August 8, 2025 16:05
Comment thread .github/workflows/check_formatting.yaml
Comment thread .github/workflows/check_formatting.yaml Outdated
zachcran
zachcran previously approved these changes Aug 8, 2025

@zachcran zachcran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I left some suggestions, but nothing necessary to block merging, so I am approving and leaving it up to you if you implement the suggestions or not.

zachcran
zachcran previously approved these changes Aug 8, 2025
@ryanmrichard ryanmrichard merged commit 3751938 into master Aug 11, 2025
5 checks passed
@ryanmrichard ryanmrichard deleted the pre-commit_update branch August 11, 2025 16:30
@github-actions

Copy link
Copy Markdown
Contributor

🚀 [bumpr] Bumped!
New version:v0.3.67
Changes:v0.3.66...v0.3.67

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.

3 participants