-
Notifications
You must be signed in to change notification settings - Fork 67
feat: Update pre-commit mirror script #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
|
||
- uses: actions/setup-python@v4 | ||
with: | ||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include an assertion message that would help us diagnose this failed assertion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this behavior retained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I updated to retain this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks great
Summary
pre-commit-mirror-maker
is not suitable for generating multiple hooks in single.pre-commit-hooks.yaml
file.Test Plan
pyproject.toml
'sruff==0.0.290
intoruff==0.0.289
, and run the script.