Add pep8-naming and more pre-commit hooks

MarkKoz commented 4 years ago

Relevant Issues

python-discord/organisation#138 python-discord/organisation#153 Closes https://github.com/python-discord/django-simple-bulma/issues/40

Description

New hooks were added for pre-commit and they will run in CI too. pep8-naming was added as a flake8 plugin to ensure names comply with PEP 8.

Hooks added

A couple of these hooks automatically apply fixes. However, they still report failure and leave any changes they make uncommitted. Therefore, the user has to commit the automatic fixes.

check-merge-conflict - Check for files that contain merge conflict strings.
check-yaml - Attempts to load all yaml files to verify syntax.
end-of-file-fixer - Makes sure files end in a newline and only a newline.
mixed-line-ending - Replaces mixed line endings with LF.
trailing-whitespace - Trims trailing whitespace.
python-check-blanket-noqa - Enforce that noqa annotations always occur with specific codes

Reasoning

Dev dependencies were updated and re-pinned because why not? They were getting old enough. I'm just following along with the other repos.
The pre-commit environment is cached to speed up builds.
To reduce redundancy and overhead in maintain dependencies, flake8 is now invoked through the current active Python environment rather than being installed in pre-commit's isolated environment. This means the hook now assumes the user already has flake8 installed i.e. via pip install -e .[dev].

Additional Details

The pre-commit venv is cached. There should only be a cache miss if the .pre-commit-config.yaml file changes or if the location of the Python interpreter changes (more realistically, if the Python version changes). Maybe Azure wipes the cache sometimes too, but in that case pre-commit will simply re-create the environment.

lemonsaurus commented 4 years ago

It would be nice to get this solved before we release v2.0.0.

I'm a little bit on the fence about it. On one hand, this has been convenient in other projects where it has been added. On the other hand, it's a little less applicable to this project (for example, we don't have any yaml files). More importantly, this PR was written before we switched to GitHub Actions, and has to be completely rewritten.

I guess I'd be open to a rewrite, but I don't really want to spend time on doing one myself. I think I could live without pre-commit in this project. We don't have a lot of contributors, and the few we have are quite experienced developers.

@MarkKoz, how do you feel about this?

MarkKoz commented 4 years ago

I don't mind re-doing it in GH Actions @lemonsaurus. Also, we do have YAML files in the .github folder.

lemonsaurus commented 4 years ago

@MarkKoz that's true. Okay, if you'd like to re-do it for GHA I will gladly review it right away.

MarkKoz commented 4 years ago

@lemonsaurus it's ready for a review now

lemonsaurus commented 4 years ago

@MarkKoz Now that we have merged the submodule refactor #56, this PR has a ton of conflicts. Could you clean up the branch so that it doesn't try to change any of the js, css, or sass files (which are no longer in this repo), and just have this PR contain the CI changes?

lemonsaurus / django-simple-bulma