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
Add pre-commit hook #333
Add pre-commit hook #333
Conversation
See glotzerlab/signac-docs#92 for how to setup pre-commit |
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! I was able to install this with requirements-precommit.txt. I tested adding/committing a bad file with trailing whitespace and unused imports and the precommit tests failed when I tried to commit. This is definitely a great idea for keeping code clean and standard! :)
One question I have is why use a separate requirements file? It might be better to have the precommit requirements included with dev because I imagine we would want all developers to follow flake8.
@jennyfothergill I agree but the developers use |
@jennyfothergill Good question! The reason we split it is so that our CI doesn't have to install all the dependencies just to run the pre-checks. It speeds things up. |
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! Nice work @kidrahahjo.
* Add pre-commit hook * Edit circleci config file, add another file requirements-precommit.txt * update changelog * remove pre-check dependencies from requirements-dev.txt
* Add pre-commit hook * Edit circleci config file, add another file requirements-precommit.txt * update changelog * remove pre-check dependencies from requirements-dev.txt
Description
Enabling the support of pre-commit for pre-commit hook.
Developers will now have to perform these steps in order to enable the pre-commit hook.
This will add the hook to
git
.Motivation and Context
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist:
If necessary: