feat: Update pre-commit mirror script#53
Conversation
|
Nice, thank you so much! We'd just discussed doing this internally, you read our minds... I will defer to @zanieb for review. (Zanie's out on vacation so will probably get to this next week.) |
zanieb
left a comment
There was a problem hiding this comment.
Thanks for contributing! I was just complaining about the mirror not working with the new hook :) I have a few questions.
.github/workflows/main.yml
Outdated
| python-version: "3.x" | ||
|
|
||
| - run: pip install pre-commit-mirror-maker | ||
| - run: pip install "urllib3>=2" tomli packaging |
There was a problem hiding this comment.
Perhaps we should add a requirements-dev.txt for these with pinned versions? e.g. we use pip-compile over in ruff-lsp (https://github.com/astral-sh/ruff-lsp).
There was a problem hiding this comment.
I listed dependencies through optional-dependencies in pyproject.toml and create a requirements-dev.txt file through pip-tools.
The setup-python action now installs the packages via the generated requirements-dev.txt file and runs the mirror.py script.
mirror.py
Outdated
| import typing | ||
| from pathlib import Path | ||
|
|
||
| import tomli |
There was a problem hiding this comment.
Let's just install Python 3.11 at https://github.com/astral-sh/ruff-pre-commit/pull/53/files#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R21 and use the built-in toml library?
There was a problem hiding this comment.
I modified the setup-python action usage, and replaced to use built-in tomllib module.
mirror.py
Outdated
| def get_current_version(pyproject: dict) -> Version: | ||
| requirements = [Requirement(d) for d in pyproject["project"]["dependencies"]] | ||
| requirement = next((r for r in requirements if r.name == "ruff"), None) | ||
| assert requirement is not None |
There was a problem hiding this comment.
Should we include an assertion message that would help us diagnose this failed assertion?
There was a problem hiding this comment.
I added some assertion messages, but not sure if I need to supplement.
| # Only commit on change. | ||
| # https://stackoverflow.com/a/9393642/3549270 | ||
| if check_output(["git", "status", "-s"]).strip(): |
There was a problem hiding this comment.
Yes! I updated to retain this behavior.
Summary
pre-commit-mirror-makeris not suitable for generating multiple hooks in single.pre-commit-hooks.yamlfile.Test Plan
pyproject.toml'sruff==0.0.290intoruff==0.0.289, and run the script.