Use uv for dependency handling#817
Conversation
WalkthroughThe codebase and CI workflows have been updated to standardize the use of the Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Runner as Workflow Runner
participant uv as uv Tool
participant Python as Python Environment
CI->>Runner: Start workflow job
Runner->>uv: Install uv via script
uv->>Runner: uv installed and PATH updated
Runner->>uv: Create virtual environment / sync dependencies
uv->>Python: Setup environment, install dependencies
Runner->>uv: Run commands (test, lint, build, etc.)
uv->>Python: Execute tasks in managed environment
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected related to the linked issue #682. Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
176bda2 to
24c5e7b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
pyproject.toml (1)
49-59: Dev‐dependencies configuration looks solid.
The new[tool.uv]section correctly declares your development tools and minimum versions for pytest, flake8, pylint, etc.Consider pinning to patch versions (e.g.
pytest==6.2.5) or generating auv.lockto ensure fully reproducible CI builds..github/workflows/pylint.yml (1)
22-25:uvinstallation step is in place.
The curl-and-sh snippet correctly installsuvand updatesPATHbefore running linting tasks.You might simplify maintenance by invoking a reusable action or composite step for
uvinstallation across all workflows..github/workflows/make.yml (1)
26-29: Consistentuvsetup in CI.
The newInstall uvstep mirrors other workflows and ensuresmaketargets leverageuv.Consider extracting this into a reusable GitHub Action or workflow template to DRY up multiple workflow files.
.github/workflows/e2e-release.yml (3)
6-6: Use unquotedon:key for workflow triggers.
While YAML allows quoted keys, GitHub Actions examples and linters expecton:without quotes for consistency. Consider changing'on':toon:.
26-30: Add a verification step foruvinstallation.
After running the install script, include auv --versioncheck to confirm the CLI is onPATHand the expected version is installed. For example:- name: Verify uv installation run: uv --version
26-28: Pin or validate theuvinstall script.
Fetching the installer unpinned can introduce breaking changes. Consider pinning to a specificuvrelease or verifying the script’s checksum to avoid unexpected updates.README.md (2)
19-23: Includeuvinstallation instructions.
You recommend usinguvbut don’t show how to install it. Add an install snippet (e.g.curl -LsSf https://astral.sh/uv/install.sh | sh) in the prerequisites or setup section for clarity.
323-326: Unifypipvspip3usage.
The README alternates betweenpip3andpip. For consistency, standardize on one (e.g., alwayspip3) across all code blocks.test/e2e/test_e2e_release.sh (5)
1-3: Useenv-based shebang for portability.
Switch to#!/usr/bin/env bashto ensure the script runs correctly across environments wherebashmay not be in/bin.
5-7: Consolidate error-handling flags.
Combineset -eandset -o pipefail(and considerset -u) into a singleset -euo pipefailfor stricter script safety.
19-23: Usepushd/popdfor directory changes.
Replacingcd "$TEMP_VENV"/cd -withpushd "$TEMP_VENV"/popdcan make context switches clearer and avoid side effects.
60-68: DRY exit-code assertions.
The pattern ofset +e, capturing$?, then reverting toset -erepeats. Consider a helper function (e.g.,assert_exit_code cmd expected) to reduce duplication and improve readability.
91-93: Explicitly assert help command exit code.
Afteraibolit check --help, you rely onset -ebut don’t verify the exit code. Add a check (if [ $? -ne 0 ] …) to ensure the help invocation succeeds as expected.Makefile (2)
26-29: Unify integration test invocation.
Instead ofuv run python -m test.integration…, consider usinguv run pytest test/integrationfor consistency with thetesttarget.
16-18: Validate thatuv.lockis up-to-date.
uv syncupdates dependencies but doesn’t enforce the lockfile’s state. Consider adding a check or informing maintainers to commituv.lockwhen dependencies change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/e2e-release.yml(1 hunks).github/workflows/make.yml(1 hunks).github/workflows/pylint.yml(1 hunks).gitignore(1 hunks)Makefile(1 hunks)README.md(2 hunks)pyproject.toml(1 hunks)test/e2e/__init__.py(1 hunks)test/e2e/test_e2e_release.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/e2e/test_e2e_release.sh (1)
aibolit/__main__.py (1)
check(710-799)
🔇 Additional comments (3)
test/e2e/__init__.py (1)
1-2: Package marker file is correct.
Adding the SPDX header in__init__.pymakestest/e2ea proper Python package and has no side effects.README.md (1)
13-14: Ensure Python version consistency across docs.
You raised the requirement to Python 3.11+, but verify all CI workflows and examples (e.g., in.github/workflows) reference 3.11+ instead of older versions.Makefile (1)
10-10: Defaultalltarget omitse2e.
The new end-to-end tests (e2e) aren’t part of thealltarget. Please confirm if this is intentional or ifallshould rune2eas well.
|
need to patch a .rultor.yml |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.rultor.yml (1)
26-27: Evaluate version‐check commands’ necessity
Running bothuv run python --versionanduv --versionadds noise to the release logs. Consider removing or gating these checks (e.g., only on a debug flag) to reduce log clutter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.rultor.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: latexmk
- GitHub Check: make
🔇 Additional comments (3)
.rultor.yml (3)
18-18: Theexport PATHin the merge stage is necessary to exposeuvfor downstream commands and is correctly scoped within the script block.
22-22: Re-exportingPATHat the start of the release stage is expected (each block runs in a fresh shell). No changes needed.
29-30: Verify build artifacts and token path
Ensure thatuv buildemits artifacts underdist/and that thetwine_tokenasset is indeed accessible via../twine_token. Also consider adding a pre-upload validation step such asuv run twine check dist/*to catch packaging metadata issues early.
| curl -LsSf https://astral.sh/uv/install.sh | sh | ||
| source $HOME/.local/bin/env | ||
| export PATH="$HOME/.local/bin:$PATH" | ||
| uv tool install twine |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin and verify the uv installer for reproducibility
The script pulls the latest uv installer unpinned, which can break future CI runs if a breaking change is published. Recommend specifying a fixed release (or channel) and verifying the installer SHA256 before execution.
🤖 Prompt for AI Agents
In .rultor.yml around lines 12 to 15, the script downloads the latest unpinned
uv installer, risking future CI breakage. Modify the curl command to download a
specific pinned version or channel of the uv installer, then verify the
installer's SHA256 checksum before running it. This ensures reproducibility and
prevents unexpected changes from breaking the CI pipeline.
ivanovmg
left a comment
ivanovmg
left a comment
There was a problem hiding this comment.
@AntonProkopyev thank you for this PR! I believe it is a solid PR targeting transition towards the more modern tools.
I have reservations against including e2e-release workflow in this PR, as it seems to be a complete different feature. It is totally useful, but looks like it deserves another task and PR.
bcc0765 to
e60974a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ty.yml (1)
21-24: Shorten thecurlcommand for YAML compliance and parameterize the version.
Line 23 exceeds the 80-characteryamllintlimit. Break the installer URL across lines with backslashes or disable theline-lengthrule on this line (# yamllint disable-line rule:line-length). For easier maintenance, consider defining theuvversion as anenvvariable (e.g.UV_VERSION: 0.7.13) at the top of the workflow instead of hardcoding it.🧰 Tools
🪛 GitHub Actions: yamllint
[error] 23-23: YAML lint error: line too long (100 > 80 characters) (line-length)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/workflows/e2e-release.yml(1 hunks).github/workflows/make.yml(1 hunks).github/workflows/pylint.yml(1 hunks).github/workflows/ty.yml(1 hunks).rultor.yml(2 hunks)Makefile(2 hunks)README.md(2 hunks)REUSE.toml(1 hunks)pyproject.toml(1 hunks)test/e2e/test_e2e_release.sh(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- REUSE.toml
🚧 Files skipped from review as they are similar to previous changes (8)
- .github/workflows/e2e-release.yml
- .github/workflows/pylint.yml
- .github/workflows/make.yml
- .rultor.yml
- pyproject.toml
- test/e2e/test_e2e_release.sh
- README.md
- Makefile
🧰 Additional context used
🪛 GitHub Actions: yamllint
.github/workflows/ty.yml
[warning] 6-6: YAML lint warning: truthy value should be one of [false, true] (truthy)
[error] 23-23: YAML lint error: line too long (100 > 80 characters) (line-length)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: latexmk
- GitHub Check: make
|
@AntonProkopyev are you still working on it? |
|
@ivanovmg Yes, I do. Will ship till end of week. |
cfe5095 to
27150f1
Compare
|
@ivanovmg Could you take a look please? Rebased branch. Also fixed dependencies more precisely. |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/e2e-release.yml(1 hunks).github/workflows/make.yml(1 hunks).github/workflows/pylint.yml(1 hunks).github/workflows/ty.yml(1 hunks).rultor.yml(2 hunks)Makefile(2 hunks)README.md(2 hunks)REUSE.toml(1 hunks)pyproject.toml(2 hunks)requirements.txt(0 hunks)test/e2e/test_e2e_release.sh(2 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🚧 Files skipped from review as they are similar to previous changes (9)
- README.md
- REUSE.toml
- .github/workflows/e2e-release.yml
- .github/workflows/pylint.yml
- .github/workflows/ty.yml
- .github/workflows/make.yml
- .rultor.yml
- test/e2e/test_e2e_release.sh
- Makefile
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: make
- GitHub Check: latexmk
🔇 Additional comments (1)
pyproject.toml (1)
26-27: No NumPy<2 constraint in NetworkX 3.2 – compatible with NumPy 2.xNetworkX 3.2’s PyPI metadata lists only “numpy >=1.22” (no upper bound), so using
numpy==2.2.5satisfies its requirement. No changes needed.
|
@yegor256 Could you take a look please? |
|
@ivanovmg are we good here? |
Co-authored-by: Maxim Ivanov <41443370+ivanovmg@users.noreply.github.com>
ivanovmg
left a comment
ivanovmg
left a comment
There was a problem hiding this comment.
@AntonProkopyev a minor comment from my side.
Otherwise, looks good!
@ivanovmg Thanks for review! Fixed. |
|
@yegor256 All issues were fixed here. Could you merge please? |
|
@rultor merge |
@AntonProkopyev @yegor256 Can't merge this pull requests, because the file |
|
@AntonProkopyev thanks! |
|
@AntonProkopyev now |
|
@AntonProkopyev Thanks for your contribution! 🚀 You've earned +4 points, which is the minimum reward as per our policy. Here's the breakdown: +16 base points; -8 for exceeding 200 hits-of-code (1770); -16 for surpassing 800 hits-of-code; -16 for no code review; -6 for 31 comments (>8) during review; +34 adjustment to reach the minimum. While the contribution was substantial, remember that our policy emphasizes quality over quantity. For future submissions, consider smaller, more focused changes and engage in code reviews. Your dedication is appreciated, and your running balance is now +108. Keep improving and contributing! |
5 participants
Closes #682
Summary by CodeRabbit
New Features
uvtool across development, testing, and release processes.Improvements
uvfor dependency management, testing, linting, and building.uvfor virtual environment creation and package installation.uvrequirements.Chores
requirements.txtfile, consolidating dependency management.uv.lockto REUSE annotations.