client_python icon indicating copy to clipboard operation
client_python copied to clipboard

Run linters and formatters via `pre-commit`

Open JoshKarpel opened this issue 3 years ago • 1 comments

Hi again!

While working on https://github.com/prometheus/client_python/pull/794 I had a few flake8 linting errors locally that I didn't catch before pushing and had to push again to fix. On other projects I've found pre-commit very helpful for preventing these errors by forcing linters and formatters to run before committing. This PR adds a pre-commit configuration which includes the existing flake8 and isort checks, plus some basic formatters and linters suggested by pre-commit. I've updated CONTRIBUTING.md to include instructions for setting up pre-commit locally and installing the hooks.

In this initial take on the PR I've left flake8 and isort running through tox as well, but there's a CI service associated with pre-commit at https://pre-commit.ci/ that I like (free for open source projects). It runs pre-commit checks on PRs and sends a PR every week to update the pre-commit check dependencies. It's also possible to run the pre-commit checks in CircleCI, example config here. I'd recommend removing the tox configuration for running flake8 and isort and running them through pre-commit instead, either through pre-commit.ci or CircleCI - your preference, assuming you're interested in this :)

JoshKarpel avatar Apr 09 '22 21:04 JoshKarpel

Personally, I am not a huge fan of pre-commit hooks as they add more overhead than I would like for some changes, or when I just want to push my progress somewhere. Yes --no-verify is an option, but I have still found them a bit jarring.

Fair point! However, this set of checks is pretty fast.

At least on my computer, running these pre-commit checks on all files (which is the absolute maximum, since during a commit they'll only run on files in the diff) takes ~4 seconds

❯ time pre-commit run --all
check for added large files..............................................Passed
check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check for case conflicts.................................................Passed
check docstring is first.................................................Passed
check for merge conflicts................................................Passed
check toml...........................................(no files to check)Skipped
check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Passed
flake8...................................................................Passed
isort....................................................................Passed

real    0m4.080s
user    0m8.024s
sys     0m1.443s

I'm running through WSL so I suspect this is longer than average due to how slow WSL is with filesystem operations. In my experience, those few seconds are worth it to not have to wait for CI to fail and do a whole extra commit + push to fix the typos.

JoshKarpel avatar Apr 12 '22 23:04 JoshKarpel