-
Notifications
You must be signed in to change notification settings - Fork 19
Upstream pre commit config #15
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
Conversation
Thanks! Will take a look soon. |
README.rst
Outdated
@@ -62,8 +62,7 @@ You need to have precommit setup to use this hook. | |||
:: | |||
|
|||
pip install pre-commit | |||
pre-commit install | |||
pre-commit install --hook-type commit-msg | |||
pre-commit install --install-hooks |
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.
This won't work. Need to explicitly run pre-commit install --hook-type commit-msg
otherwise giticket wont work. needs to be reverted
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.
Looks like you're right. I expected that --install-hooks was minimally sufficient, but it doesn't actually install any hooks, just sets up the envs for the existing hooks.
CONTRIBUTING.rst
Outdated
4. Set up pre-commit:: | ||
|
||
$ pip install pre-commit | ||
$ pre-commit install --install-hooks |
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.
Would add pre-commit install --hook-type commit-msg
step here as well
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.
Need to mention running pre-commit install --hook-type commit-msg
for new users.
I've restored the original instructions, and filed pre-commit/pre-commit#1895 to have |
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.
Thanks for the changes! Looks great
This PR sets up pre-commit on the repo itself. It enables a few simple tests, and then turns on flake8 on commit.
Docs are also updated to simplify the installation, and to instruct developers on how to set up pre-commit themselves.