-
Notifications
You must be signed in to change notification settings - Fork 230
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
ruff instead of black / isort #541
Conversation
run: black --check --preview . | ||
- name: isort check | ||
run: isort --profile black --check . | ||
curl -LsSf https://astral.sh/uv/install.sh | sh |
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.
It's a good practice to verify the integrity of the script before executing it. Consider adding a checksum verification step for the install.sh
script to ensure its authenticity and integrity.
run: isort --profile black --check . | ||
curl -LsSf https://astral.sh/uv/install.sh | sh | ||
uv venv | ||
uv pip install -r dev-requirements.txt |
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.
Using uv pip install
directly might not be clear to everyone. Consider adding a comment explaining that uv
is a tool for managing Python environments and that this command installs dependencies in a virtual environment.
- name: activate venv | ||
run: | | ||
. .venv/bin/activate | ||
echo PATH=$PATH >> $GITHUB_ENV |
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.
Appending to the PATH
environment variable directly in the script can lead to unexpected behavior in complex workflows. If possible, consider using environment files or other GitHub Actions features for setting environment variables.
- name: ruff (linter) | ||
run: ruff check . | ||
- name: ruff (formatter) | ||
run: ruff format --check . |
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.
The ruff format --check .
command is used to check formatting without making changes. If the intent is to also format the code, consider running ruff format
without the --check
flag in a separate step that only runs on a specific branch or conditionally, to avoid unintended code changes in pull requests.
MENTAT CODE REVIEW IN ACTIVE DEVELOPMENT. Only in use on mentat and internal repos. The switch to using |
c7a15fb
to
04c8f20
Compare
04c8f20
to
2379430
Compare
Pull Request Checklist