-
Notifications
You must be signed in to change notification settings - Fork 10
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
chore: fix upgrade-dependencies.yaml
docs constraints
#1215
Conversation
Reviewer's Guide by SourceryThis pull request updates the Python version and output file name in the dependency upgrade workflow for documentation-related constraints. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe pull request modifies the workflow file Changes
Possibly related PRs
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Quality Gate passedIssues Measures |
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.
Hey @Czaki - I've reviewed your changes - here's some feedback:
Overall Comments:
- There's a typo in the commit message: 'dependecies' should be 'dependencies'. Consider correcting this for clarity in the repository history.
- Before merging, it's recommended to run a test build of the documentation using Python 3.12 to ensure there are no unexpected issues with the new version.
- Don't forget to update any other references to Python 3.11 in the project documentation or configuration files, if any exist, to maintain consistency with this change.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
🧹 Outside diff range and nitpick comments (1)
.github/workflows/upgrade-dependencies.yml (1)
Line range hint
1-77
: Consider improving workflow maintainabilityWhile the current change is correct, here are some suggestions to improve the overall maintainability of the workflow:
Use a variable for the latest Python version: Instead of hardcoding "3.12" in multiple places, consider using a GitHub Actions variable or environment variable. This would make future version updates easier.
Dynamically generate Python version list: Instead of manually listing Python versions, consider using a range or array that can be easily updated.
Parameterize the documentation constraints generation: Make the documentation constraints generation more flexible by using the same loop as the main constraints, allowing for easy addition of new Python versions.
Here's an example of how you could implement these suggestions:
env: LATEST_PYTHON: "3.12" PYTHON_VERSIONS: "3.9 3.10 3.11 3.12" steps: - uses: actions/setup-python@v5 with: python-version: ${{ env.LATEST_PYTHON }} cache: pip cache-dependency-path: 'pyproject.toml' - name: Upgrade Python dependencies run: | set -x pip install -U uv flags=(--extra pyqt6 --extra pyside2 --extra pyside6 --extra test --extra pyinstaller_base) for pyv in ${{ env.PYTHON_VERSIONS }}; do uv pip compile --python-version ${pyv} --upgrade --output-file requirements/constraints_py${pyv}.txt pyproject.toml requirements/version_denylist.txt "${flags[@]}" uv pip compile --python-version ${pyv} --upgrade --output-file requirements/constraints_py${pyv}_pydantic_1.txt pyproject.toml requirements/version_denylist.txt "${flags[@]}" --constraint requirements/pydantic_1.txt if [ "${pyv}" = "${{ env.LATEST_PYTHON }}" ]; then uv pip compile --python-version ${pyv} --upgrade --output-file requirements/constraints_py${pyv}_docs.txt pyproject.toml --extra docs --extra pyqt6 fi doneThese changes would make the workflow more maintainable and easier to update in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/upgrade-dependencies.yml (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
.github/workflows/upgrade-dependencies.yml (1)
42-42
: LGTM! Verify documentation build with Python 3.12The update to use Python 3.12 for compiling documentation dependencies is consistent with the Python version specified in the
actions/setup-python
step. This change ensures that the documentation constraints are generated using the latest Python version.To ensure this change doesn't introduce any issues, please verify that the documentation builds successfully with Python 3.12. You may want to run the documentation build process locally or in a separate CI job to confirm compatibility.
upgrade-dependecies.tml
docs constraintsupgrade-dependencies.yaml
docs constraints
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1215 +/- ##
===========================================
- Coverage 93.15% 93.13% -0.02%
===========================================
Files 210 210
Lines 33169 33169
===========================================
- Hits 30898 30893 -5
- Misses 2271 2276 +5 ☔ View full report in Codecov by Sentry. |
Summary by Sourcery
CI:
Summary by CodeRabbit