Fixed #1199 -- Added pre-commit hooks #1198
Conversation
1df202f to
d9a6a9f
Compare
There was a problem hiding this comment.
Hi @pauloxnet
Thank you for the PR. Overall, this looks good to me.
Please find the review inline. Curious to know what are your thoughts on adding black in pre-commit hooks.
| - repo: https://github.com/pycqa/isort | ||
| rev: "5.10.1" | ||
| hooks: | ||
| - id: isort |
There was a problem hiding this comment.
Do you think adding black along with isort would make sense here? It would provide a consistent formatting across the project.
Please let me know your thoughts.
There was a problem hiding this comment.
There is a specific issue to format the code base with Black so I would defer adding black in pre-commit to that scope.
#1184
There was a problem hiding this comment.
Alright, that sounds good to me!
| On the first commit ``pre-commit`` will install the hooks, these are | ||
| installed in their own environments and will take a short while to | ||
| install on the first run. Subsequent checks will be significantly faster. | ||
| If the an error is found an appropriate error message will be displayed. | ||
| If the error was with ``isort`` then the tool will go ahead and fix them for | ||
| you. Review the changes and re-stage for commit if you are happy with | ||
| them. |
There was a problem hiding this comment.
| On the first commit ``pre-commit`` will install the hooks, these are | |
| installed in their own environments and will take a short while to | |
| install on the first run. Subsequent checks will be significantly faster. | |
| If the an error is found an appropriate error message will be displayed. | |
| If the error was with ``isort`` then the tool will go ahead and fix them for | |
| you. Review the changes and re-stage for commit if you are happy with | |
| them. | |
| On the first commit ``pre-commit`` will install the hooks, these are | |
| installed in their environments and will take a short while to | |
| install on the first run. Subsequent checks will be significantly faster. | |
| If an error is found an appropriate error message will be displayed. | |
| If the error was with ``isort`` then the tool will go ahead and fix them for | |
| you. Review the changes and re-stage for commit if you are happy with | |
| them. |
There was a problem hiding this comment.
I copied this paragraph from the Django code, so I'd rather it stay aligned here too, or at most I'd make your changes in both repositories.
https://github.com/django/django/blob/main/docs/internals/contributing/writing-code/coding-style.txt#L9
There was a problem hiding this comment.
Okay. @carltongibson what are your thoughts?
97baf98 to
a76fca4
Compare
|
I rebased this PR and solved the conflicts. I think it's ready to be merged. |
* Added pre-commit with a few hooks * Add pre-commit instructions in README
As a consequence of the comments relating to the old pull request, I created a new pull request to add pre-commit.
The configuration file now has only basic hooks with the exclusion of migrations files.