Pre-commit demo #187
Pre-commit demo #187
Conversation
There was a problem hiding this comment.
This is the configuration that will differ across repos.
| @@ -0,0 +1,21 @@ | |||
| --- | |||
| repos: | |||
| - repo: "https://github.com/astral-sh/ruff-pre-commit" | |||
There was a problem hiding this comment.
I switched from black + pyflakes + isort to ruff, which is the current state-of-the-art for python linting and formatting. It's written in rust, opinionated in mostly sane ways, and is hella fast.
| hooks: | ||
| # Run the linter. | ||
| - id: "ruff" | ||
| exclude: ".*_pb2.*" |
There was a problem hiding this comment.
Ignore the generated files
| # Run the formatter. | ||
| - id: "ruff-format" | ||
| exclude: ".*_pb2.*" | ||
| - repo: "https://github.com/adrienverge/yamllint" |
There was a problem hiding this comment.
We can run yamllint through here as well
| rev: "v1.35.1" | ||
| hooks: | ||
| - id: "yamllint" | ||
| - repo: "https://github.com/RobertCraigie/pyright-python" |
There was a problem hiding this comment.
I also swapped from mypy to pyright - pyright is the one that vscode uses, and it's got some performance and typing benefits over mypy. There's a comparison here: https://github.com/microsoft/pyright/blob/main/docs/mypy-comparison.md
| @@ -1,3 +1,9 @@ | |||
| [virtualenvs] | |||
There was a problem hiding this comment.
This was necessary to get pyright via pre-commit to play nicely with poetry. We store the venv locally (kinda like having a local node_modules) and then pyright can reference it.
| # NOTE: this isn't ideal. The issue here appears to be with the code that's | ||
| # generated for google grpc Statuses, and pyright complains that Status isn't | ||
| # on the module, which *does* appear to be true. It's unclear whether this is | ||
| # an issue with our usage or with the package that generates types. | ||
| # TODO: try using pyi to generate the stubs rather than mypy | ||
| reportAttributeAccessIssue = "warning" |
There was a problem hiding this comment.
Just calling this out.
.github/workflows/lint.yaml
Outdated
| - name: "Install poetry" | ||
| run: "pipx install poetry" | ||
| - uses: "actions/setup-python@v5" |
There was a problem hiding this comment.
This makes deps available for pyright, and should also do a better job of caching deps between runs.
| - name: "Install Poetry" | ||
| uses: "snok/install-poetry@v1" | ||
| with: | ||
| config-file: ".yamllint" | ||
| # This should come from pyproject.toml but poetry | ||
| # wasn't picking it up for whatever reason. | ||
| virtualenvs-in-project: true | ||
| virtualenvs-path: ".venv" |
There was a problem hiding this comment.
I'm not super happy about this, but poetry doesn't otherwise seem to properly respect the pyproject.toml settings for this.
There was a problem hiding this comment.
I'm having the issues with pyright you mentioned, no matter how many times I removed and recreated my poetry venv. Not sure if it has to do with PyCharm's poetry support. The other linters seem to pass, though.
I missed instructions on how to install pre-commit. I resourced to brew install pre-commit which brought a fair share of system-wide updates. I'd suggest adding a new section in the README that describes how to setup the development environment.
I like the idea of pre-commit hooks to align how linters are run across the projects, although I'd admit I'm not a fan of putting another tool in the critical (development) path (I learned that "git pre-commit hooks" and pre-commit are different things)
I left some questions as well.
| push: | ||
| branches: | ||
| - "main" | ||
| pull_request: | ||
| branches: ["*"] | ||
| jobs: | ||
| lint: | ||
| name: "Format & Lint" |
There was a problem hiding this comment.
why removal of the name?
There was a problem hiding this comment.
No strong reason - it was when i was deleting and replacing chunks of the configuration. I can reinstate it.
Description
This is a POC of moving linting concerns into pre-commit across our various client repositories. Ideally this should centralize linting configuration and make it easier to get linting working on a particular repo.
Changes
Gonna annotate. There's a few of them.
Testing
Review, see that the lint action passes and does what you need it to do. Run
pre-commit run -alocally and see that it succeeds. If you get errors about pyright not being able to find definitions, you might have to remove your poetry env and re-install so that it installs the venv locally in a place where pyright can find it.