Hi David

I like pre-commit too. I think it would be a great addition. Your initial
setup looks good - and perhaps we can add a few more hooks from
pre-commit-hooks like check-case-conflict and check-json.

One thing I prefer to do is use pre-commit to run binaries through "system"
(e.g. [1]). This allows pinning through requirements.txt, reduces the
number of virtualenvs, and reduces the number of places versions can be
pinned. I'm open on which way we do this, but thought I'd share my solution.

Final note is that while thinking about how to flag this to users I
> wondered if a pull request template would be helpful, with the checklist in
> it, and a notice about pre-commit?
>

If pre-commit runs on CI this may just be noise. But open to see it - we
don't have any PR template at the moment, and it could be helpful to point
people to Trac and the contribution guide at least.

[1]:
https://github.com/adamchainz/apig-wsgi/blob/master/.pre-commit-config.yaml



On Fri, 6 Nov 2020 at 08:26, [email protected] <[email protected]> wrote:

> Hi All
>
> I have opened a ticket[1]  which proposes to add pre-commit[2] to Django,
> and wish to gather thoughts on it here.
>
> For anyone not familiar pre-commit checks for simple issues before each
> `git commit`. The aim is to try and catch format errors closer to the
> developer and therefore reduce C/I tests that need to be re-run.
>
> I propose that we add pre-commit hooks for the existing code style checks
> (flake8, isort) and to check for the whitespace rule. In addition, we can
> add a hook for an empty last line. My thought on an initial setup is
> something like this[3].
>
> Documentation would be added as part of the guide to getting set up with
> the Django test suite [4] and would include notes on how to install the
> pre-commit hooks. I also think a smaller note with the patch review
> checklist could also be useful.
>
> Looking ahead I think this tool would be very useful to have in place for
> when Black (DEP8) is added as it will be much more strict on code style.
> This will result
> in far more pull requests being opened and commits being pushed which will
> fail the code style checks quickly. If we can reduce the number of times
> this happens
> then I think this is a good change.
>
> Final note is that while thinking about how to flag this to users I
> wondered if a pull request template would be helpful, with the checklist in
> it, and a notice about pre-commit?
>
> Kind Regards
>
> David
>
> [1] - https://code.djangoproject.com/ticket/32165
> [2] - https://pre-commit.com/
> [3] - https://gist.github.com/smithdc1/66e6f696beec25e47fdbbef4bca4ab34
> [4] -
> https://docs.djangoproject.com/en/3.1/intro/contributing/#previewing-your-changes
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/c114c95a-e9a4-4b25-8a6b-54ae4bac9b22n%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/c114c95a-e9a4-4b25-8a6b-54ae4bac9b22n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>


-- 
Adam

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM19fVnGi6NVQK44cTftzx0GOOtj7VH8nRhaq50T-R1grQ%40mail.gmail.com.

Reply via email to