Page MenuHomePhabricator

[dev] pre-commit for linting and code formatting
Open, Needs TriagePublic

Description

I suggest moving Pywikibot's linters/code formatters to pre-commit.
pre-commit runs configured hooks (.pre-commit-config.yaml) against files changed in a commit when you git commit .... (No more forgetting to run checks before sending for review.) It would also be run against all files in CI in case someone doesn't install it locally.

https://gerrit.wikimedia.org/r/c/pywikibot/core/+/684050 is a proof of concept patch.

git-review -d 684050        # checkout the POC patch

pip install pre-commit      # Install pre-commit

pre-commit install          # Install pre-commit's pre-commit hook
git commit ...              # pre-commit will run on the files changed
pre-commit uninstall        # Unnstall pre-commit's pre-commit hook

pre-commit run --all-files  # Run pre-commit on all files

Hooks included in the POC patch to demonstrate functionality:

Include?HookDescription
check-added-large-filesPrevent giant files from being committed.
check-astSimply check whether files parse as valid python.
check-builtin-literalsRequire literal syntax when initializing empty or zero Python builtin types.
check-docstring-firstChecks for a common error of placing code before the docstring.
check-jsonAttempts to load all json files to verify syntax.
check-merge-conflictCheck for files that contain merge conflict strings.
check-symlinksChecks for symlinks which do not point to anything.
check-tomlAttempts to load all TOML files to verify syntax.
check-vcs-permalinksEnsures that links to vcs websites are permalinks.
check-xmlAttempts to load all xml files to verify syntax.
check-yamlAttempts to load all yaml files to verify syntax.
debug-statementsCheck for debugger imports and py37+ breakpoint() calls in python source.
destroyed-symlinksDetects symlinks which are changed to regular files with a content of a path which that symlink was pointing to. This usually happens on Windows when a user clones a repository that has symlinks but they do not have the permission to create symlinks.
end-of-file-fixerMakes sure files end in a newline and only a newline.
fix-byte-order-markerremoves UTF-8 byte order marker
fix-encoding-pragmaRemove # -*- coding: utf-8 -*- from the top of python files.
forbid-new-submodulesPrevent addition of new git submodules.
mixed-line-endingReplaces or checks mixed line ending.
pretty-format-jsonChecks that all your JSON files are pretty. "Pretty" here means that keys are sorted and indented.
trailing-whitespaceTrims trailing whitespace.
remove-crlfReplace CRLF (\r\n) with LF (\n)
remove-tabsReplace tab characters with spaces
pyupgradeRun pyupgrade
isortRun isort
Yesflake8Run flake8

Please take a look and share your thoughts about using pre-commit in Pywikibot development.
If we want to use it, which hooks do we want to include?

Event Timeline

Can you please introduce a bit more of your Approach. What does this mean for code styles, styles check, the code itself, ceviewers, coders, uploaders.

JJMC89 updated the task description. (Show Details)

Can you please introduce a bit more of your Approach. What does this mean for code styles, styles check, the code itself, ceviewers, coders, uploaders.

I've expanded the description. The impact on code is dependent on which hooks are configured. The hooks in the POC patch are there to demonstrate successes and failures, not that we would necessarily need/want to use all of them.

For me, the biggest benefits are automatic running on commit and making it easier (for new coders, especially) to (automatically, in the case of formatters) fix issues before uploading.

Do I understand right: pre-commit changes the code. Is ist done during git commit or when uploading?

Do I understand right: pre-commit changes the code. Is ist done during git commit or when uploading?

It is done when git commit ... is run (or you can manually run it with pre-commit run). If any of the hooks make changes or find issues, then it prevents the commit. You would then: git diff to show changes, make any additional changes (e.g. fike flake8 issues), git add ..., and git commit ... again.