-
-
Notifications
You must be signed in to change notification settings - Fork 20
uv #194
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
WalkthroughThis pull request updates the project's dependency management and build configurations. The README now provides updated instructions for creating and activating virtual environments using the Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (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:
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 (
|
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
🧹 Nitpick comments (3)
.github/workflows/nightly.yml (1)
39-50: Verify UV setup consistency in the nightly workflow.
The nightly workflow now uses the new UV setup (lines 43–50) with commandsuv venvanduv pip sync uv.lock. Ensure that the corresponding environment (e.g. the lock file and virtual environment directory) is consistent with the dev workflow and that any caching (if later added) properly reflects the UV lock file..github/README.md (1)
46-46: Minor Markdown header punctuation note.
According to markdown lint guidelines, consider verifying if the heading"### Create and activate a virtual environment then Install project dependencies"needs any punctuation adjustments (e.g. removing possible trailing punctuation or adding a clarifying comma) to improve readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: Possible missing comma found.
Context: ... ``` ### Create and activate a virtual environment then Install project dependencies ####...(AI_HYDRA_LEO_MISSING_COMMA)
pyproject.toml (1)
40-45: Ensure build target consistency.
The new sections[tool.hatch.build.targets.sdist]and[tool.hatch.build.targets.wheel]include"playwright_python". Please verify that the package directory is indeed namedplaywright_python(using an underscore) rather than matching the hyphenated project name"playwright-python".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/README.md(1 hunks).github/workflows/devRun.yml(1 hunks).github/workflows/nightly.yml(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.github/README.md
[uncategorized] ~46-~46: Possible missing comma found.
Context: ... ``` ### Create and activate a virtual environment then Install project dependencies ####...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
.github/README.md
48-48: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
56-56: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (2)
.github/workflows/devRun.yml (1)
19-27:❓ Verification inconclusive
Ensure the new UV integration uses the correct lock file for caching.
The workflow now sets up UV and installs dependencies viauv venvanduv pip sync uv.lock, but the dependency cache step (line 40) still referencespoetry.lock. This can lead to caching mismatches. Please update the cache key to compute the hash overuv.lockinstead.- key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ hashFiles('**/poetry.lock') }} + key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ hashFiles('**/uv.lock') }}
Action Required: Update Cache Key in Workflow
After reviewing the changes introduced by the new UV integration, please note that the dependency cache step (line 40) still computes its cache key using the hash of
poetry.lockinstead of the updateduv.lock. This inconsistency could lead to caching mismatches. Kindly update the cache key as indicated below:- key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ hashFiles('**/poetry.lock') }} + key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ hashFiles('**/uv.lock') }}Please verify that all references and paths are consistent with the new UV integration.
pyproject.toml (1)
1-9: Good transition from Poetry to Hatchling.
The[build-system]and[project]sections correctly reflect the shift to Hatchling with updated dependencies and metadata. Confirm that the package inclusion in the Hatchling targets (specified as"playwright_python") exactly matches the actual source package name.
| ### Create and activate a virtual environment then Install project dependencies | ||
|
|
||
| #### For Windows: | ||
| ```bash | ||
| py -m pip install --user virtualenv | ||
| py -m venv env | ||
| pip install uv | ||
| uv venv | ||
| .\env\Scripts\activate | ||
| uv pip sync uv.lock | ||
| ``` |
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.
💡 Verification agent
❓ Verification inconclusive
Clarify and standardize virtual environment instructions.
The updated instructions correctly replace Poetry commands with UV commands. However, there is an inconsistency: the Windows instructions show activation via .\env\Scripts\activate (line 52) while the Unix/Mac instructions activate with .venv (line 60). For consistency with workflows (which use .venv), update the Windows activation command to use .venv\Scripts\activate.
- .\env\Scripts\activate
+ .\.venv\Scripts\activateActionable: Update Windows Virtual Environment Activation Command
- In the file
.github/README.md, please update the Windows activation command from:to:.\env\Scripts\activate
.\.venv\Scripts\activate
- This change ensures consistency with Unix/Mac instructions that use
.venv.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: Possible missing comma found.
Context: ... ``` ### Create and activate a virtual environment then Install project dependencies ####...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
48-48: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
Description
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit
Documentation
Chores
uvfor environment setup.