-
Notifications
You must be signed in to change notification settings - Fork 32
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
Augment .pre-commit #257
Augment .pre-commit #257
Conversation
Codecov Report
@@ Coverage Diff @@
## master #257 +/- ##
=======================================
Coverage 91.36% 91.36%
=======================================
Files 22 22
Lines 2779 2779
=======================================
Hits 2539 2539
Misses 240 240 Continue to review full report at Codecov.
|
@@ -2,6 +2,8 @@ repos: | |||
- repo: https://github.com/pre-commit/pre-commit-hooks | |||
rev: v2.5.0 | |||
hooks: | |||
- id: no-commit-to-branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood correctly, the purpose of pre-commit is to give the contributor a chance to check the code in one simple command before comitting it from the working directory to his/her local repo. From this pre-commit yaml file I only see reference to flake8
and rstcheck
. Should black --check .
and pytest tests/
also be included? These are the four tests I usually run in separate commands before I commit anything to my local repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-commit (after a local user has run pre-commit install
in his/hers local copy) will add a hook to git commit
so that these commands are checked every time a git commit
is attempted.
Black is already included, but pytest should be excluded as it generally takes too long time. pre-commit should finish in a couple of seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.