juhannc commented 1 year ago

Add pre-commit settings

Introduction and description

This PR adds adds pre-commit hooks for easier developing. The hooks can automate mundane tasks to make a developers life easier.

Notably file changes

pre-commit-config.yaml: The file containing the configuration for the hooks.
CONTRIBUTING.md: Small addition about the usage of pre-commit hooks.
setup.py: Add pre-commit as a dependency

Added hooks

Currently implemented hooks (see the list of hooks for further info and more hooks):

check-ast: simply checks whether the files parse as valid python (fails e.g. on missing brackets)
check-case-conflict: checks for files that would conflict in case-insensitive filesystems
check-docstring-first: checks a common error of defining a docstring after code
check-merge-conflict: checks for files that contain merge conflict strings (blocks a commit if it contains merge conflicts marker)
check-yaml: checks yaml files for parseable syntax
end-of-file-fixer: ensures that a file is either empty, or ends with one newline
trailing-whitespace: trims trailing whitespace

Future improvements

The following pre-commit hooks might also be worth considering:

codespell(https://github.com/codespell-project/codespell): Fixing common spelling mistakes (Note: some folders/files have to be excluded, e.g. svg files)
absolufy-imports (https://github.com/MarcoGorelli/absolufy-imports): Make python imports absolute (They are recommended by PEP8 and genereally prefered)
isort https://github.com/pycqa/isort: Sorting python imports, can be configured to follow skrls sorting style
pyupgrade https://github.com/asottile/pyupgrade: Upgrade common mistakes to a configured Python version, here 3.6

I guess the last one is currently not really needed but might be interesting if you, @Toni-SM, decide to upgrade the minimum Python version in the future. Although, it would convert the currently used format options for strings with the superior f-strings

Cheers,

Johann

Toni-SM commented 1 year ago

Nice... the only thing to say against it is that it modifies the .svg files 🙄

juhannc commented 1 year ago

Nice... the only thing to say against it is that it modifies the .svg files 🙄

They also had problems with the line endings and end of files. I looked into the raw diffs to ensure it didn't change the svg itself. I also tried out the codespell but it did in fact change the svg. Thus, if you want to include the codespell in the future to fix common typos, we'd have to take a look at what files to exclude.

Toni-SM / skrl