-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Switch to rye and ruff #36
Switch to rye and ruff #36
Conversation
Warning Rate Limit Exceeded@madeddie has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 48 minutes and 47 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThis update brings a fresh CI workflow for testing across Python versions, refines the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Feedback from Senior Dev Bot
.github/workflows/ci.yml
Outdated
--- | ||
name: CI | ||
|
||
on: | ||
push: | ||
branches: [ "main" ] | ||
pull_request: | ||
branches: [ "main" ] | ||
workflow_dispatch: | ||
|
||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
test: | ||
runs-on: ubuntu-latest | ||
|
||
strategy: | ||
matrix: | ||
python-version: ["pypy3.9", "pypy3.10", "3.8", "3.9", "3.10", "3.11", "3.12"] | ||
|
||
permissions: | ||
pull-requests: write | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: eifinger/setup-rye@v3 | ||
id: setup-rye | ||
with: | ||
enable-cache: true | ||
cache-prefix: ${{ matrix.python-version }} | ||
- name: pin python-version ${{ matrix.python-version }} | ||
if: steps.setup-rye.outputs.cache-hit != 'true' | ||
run: rye pin ${{ matrix.python-version }} | ||
- name: install dependencies | ||
if: steps.setup-rye.outputs.cache-hit != 'true' | ||
run: | | ||
rye sync --no-lock | ||
- name: run ruff | ||
uses: chartboost/ruff-action@v1 | ||
- name: run tests | ||
run: | | ||
echo '```' > pytest.log | ||
rye test | tee -a pytest.log | ||
echo '```' >> pytest.log | ||
- name: PR comment pytest output | ||
uses: thollander/actions-comment-pull-request@v2 | ||
with: | ||
filePath: pytest.log | ||
if: github.event_name == 'pull_request' |
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.
CODE REVIEW
- Improve Caching: Use the
actions/cache
for better control over caching. - Run Order:
run ruff
should depend oninstall dependencies
. - Use
venv
: Consider Python's virtual environments for isolation.
Here’s an optimized version:
- name: install dependencies
run: |
rye sync --no-lock
- name: run ruff
run: |
rye run ruff
Consider switching rye pin
and rye sync
as dependencies should be installed after pinning the Python version.
pyproject.toml
Outdated
[project] | ||
name = "tyora" | ||
version = "0.1.0" | ||
description = "CLI to interact with the mooc.fi instance of CSES" | ||
authors = [ | ||
{ name = "Edwin Hermans", email = "[email protected]" } | ||
] | ||
dependencies = [ | ||
"html2text>=2024.2.26", | ||
"htmlement>=2.0.0", | ||
"requests>=2.32.1", | ||
] | ||
readme = "README.md" | ||
requires-python = ">= 3.12" | ||
|
||
[project.scripts] | ||
"tyora" = "tyora:main" | ||
|
||
[build-system] | ||
requires = ["hatchling"] | ||
build-backend = "hatchling.build" | ||
|
||
[tool.rye] | ||
managed = true | ||
dev-dependencies = [ | ||
"pytest>=8.2.1", | ||
"requests-mock>=1.12.1", | ||
"pytest-cov>=5.0.0", | ||
] | ||
|
||
[tool.hatch.metadata] | ||
allow-direct-references = true | ||
|
||
[tool.hatch.build.targets.wheel] | ||
packages = ["src/tyora"] | ||
|
||
[tool.pytest.ini_options] | ||
addopts = "--cov" |
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.
CODE REVIEW
Your configurations look good. Here are a few suggestions:
- Add a license field to the project metadata.
license = "MIT"
- Sort dependencies alphabetically for better readability.
dependencies = [
"html2text>=2024.2.26",
"htmlement>=2.0.0",
"requests>=2.32.1",
]
- Ensure consistent formatting of authors' list (use either inline or multiline).
authors = [
{ name = "Edwin Hermans", email = "[email protected]" }
]
# python generated files | ||
__pycache__/ | ||
*.py[oc] | ||
build/ | ||
dist/ | ||
wheels/ | ||
*.egg-info | ||
|
||
# venv | ||
.venv | ||
__pycache__ | ||
|
||
# local testing objects | ||
*.json | ||
cookies.txt | ||
# testing files | ||
.coverage |
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.
CODE REVIEW
Your changes to .gitignore
provide better clarity and organization. Here are some suggestions for further improvement:
- Add comments to separate different sections clearly.
- Include
.pytest_cache/
and*.log
for more comprehensive ignore patterns.
# python generated files
+*.py[cod]
+*.pyo
# build
+build/
+dist/
+wheels/
+*.egg-info
# virtual environments
.venv/
# testing and environment files
.coverage
+pytest_cache/
+*.log
These adjustments ensure more files are appropriately ignored, maintaining a clean repository.
.pre-commit-config.yaml
Outdated
- id: check-merge-conflict | ||
- id: check-json | ||
- id: no-commit-to-branch | ||
- repo: https://github.com/psf/black | ||
rev: 22.10.0 | ||
- repo: https://github.com/astral-sh/ruff-pre-commit | ||
# Ruff version. | ||
rev: v0.4.4 | ||
hooks: | ||
- id: black | ||
# Run the linter. | ||
- id: ruff | ||
args: [ --fix ] | ||
# Run the formatter. | ||
- id: ruff-format | ||
- repo: https://github.com/pre-commit/mirrors-mypy | ||
rev: v1.10.0 | ||
hooks: |
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.
CODE REVIEW
Your changes look good, but here are some suggestions for improvement:
- Ensure consistent indentation and alignment for better readability.
- Update comments for clarity and relevance.
Here is a modified version:
- id: check-merge-conflict
- id: check-json
- id: no-commit-to-branch
- repo: https://github.com/psf/black
+ repo: https://github.com/astral-sh/ruff-pre-commit
+ # Ruff version.
+ rev: v0.4.4
hooks:
# Run the linter.
- id: ruff
args: [--fix]
# Run the formatter.
- id: ruff-format
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.10.0
hooks:
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.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
requirements-dev.lock
is excluded by!**/*.lock
requirements.lock
is excluded by!**/*.lock
Files selected for processing (5)
- .github/workflows/ci.yml (1 hunks)
- .gitignore (1 hunks)
- .pre-commit-config.yaml (1 hunks)
- .python-version (1 hunks)
- pyproject.toml (1 hunks)
Files skipped from review due to trivial changes (3)
- .github/workflows/ci.yml
- .gitignore
- .python-version
Additional comments not posted (4)
.pre-commit-config.yaml (1)
13-21
: Switch fromblack
toruff
for linting and formatting is noted. Ensure that the versionv0.4.4
ofruff
supports all the necessary features and is stable.pyproject.toml (3)
19-21
: Build system configuration usinghatchling
is appropriate for modern Python packaging.
23-38
: Tool configurations forrye
,hatch.metadata
, andpytest
are correctly set. Ensure that the development dependencies and pytest options are aligned with the project's testing strategy.
1-12
: Project metadata and dependencies are well-defined. Ensure that the specified dependencies are compatible with Python>= 3.12
.
[project.scripts] | ||
"tyora" = "tyora:main" |
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.
Tip
Codebase Verification
The tyora
module was found in src/tyora/__init__.py
and tests/test_tyora.py
, but the main
function was not located in either file. This indicates that the main
function, which is expected to be the entry point for the script, is missing.
src/tyora/__init__.py
tests/test_tyora.py
Please ensure that the main
function is implemented in the tyora
module as specified in the pyproject.toml
.
Analysis chain
Script mapping is correctly set to "tyora:main"
. Ensure that the main
function is implemented in the tyora
module.
The previous script execution failed due to a pattern syntax error in ast-grep
. We need to correct the pattern syntax and re-run the verification script to ensure the main
function exists in the tyora
module.
The previous script execution failed because the src/tyora.py
file was not found. We need to locate the correct path of the tyora
module and then verify the existence of the main
function.
The previous script execution found tests/test_tyora.py
but did not locate the main
function. We need to locate the correct implementation file for the tyora
module and verify the existence of the main
function.
The previous script execution did not find the tyora.py
implementation file. We need to locate the correct module for tyora
and verify the existence of the main
function. Let's search for any Python file that might contain the tyora
module.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the `main` function in the `tyora` module.
# Test: Search for the `main` function in the `tyora` module.
ast-grep --lang python --pattern $'def main($_) { $$$ }' src/tyora.py
Length of output: 513
Script:
#!/bin/bash
# Description: Verify the existence of the `main` function in the `tyora` module.
# Test: Search for the `main` function in the `tyora` module.
ast-grep --lang python --pattern 'def main($$):' src/tyora.py
Length of output: 123
Script:
#!/bin/bash
# Description: Locate the `tyora` module and verify the existence of the `main` function.
# Step 1: Locate the `tyora` module.
tyora_path=$(fd tyora.py)
# Step 2: Verify the existence of the `main` function in the located `tyora` module.
if [ -n "$tyora_path" ]; then
ast-grep --lang python --pattern 'def main($$):' "$tyora_path"
else
echo "tyora.py not found"
fi
Length of output: 151
Script:
#!/bin/bash
# Description: Locate the `tyora` implementation file and verify the existence of the `main` function.
# Step 1: Locate the `tyora` implementation file.
tyora_impl_path=$(fd tyora.py | grep -v tests)
# Step 2: Verify the existence of the `main` function in the located `tyora` implementation file.
if [ -n "$tyora_impl_path" ]; then
ast-grep --lang python --pattern 'def main($$):' "$tyora_impl_path"
else
echo "tyora.py implementation file not found"
fi
Length of output: 153
Script:
#!/bin/bash
# Description: Locate the `tyora` module and verify the existence of the `main` function.
# Step 1: Locate any Python file that might contain the `tyora` module.
tyora_module_path=$(fd .py | grep -i tyora)
# Step 2: Verify the existence of the `main` function in the located `tyora` module.
if [ -n "$tyora_module_path" ]; then
for file in $tyora_module_path; do
echo "Checking $file"
ast-grep --lang python --pattern 'def main($$):' "$file"
done
else
echo "No tyora module found"
fi
Length of output: 495
|
|
1 similar comment
|
|
|
|
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/ci.yml (1 hunks)
- .pre-commit-config.yaml (1 hunks)
- pyproject.toml (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci.yml
- .pre-commit-config.yaml
- pyproject.toml
|
3 similar comments
|
|
|
Summary by CodeRabbit
New Features
main
branch activities.3.12.3
for project consistency.tyora
project inpyproject.toml
.Chores
.gitignore
to exclude generated Python files, build artifacts, and testing-related files.black
toruff
for linting and formatting in.pre-commit-config.yaml
.