-
Notifications
You must be signed in to change notification settings - Fork 1
chore: switch to uv
#122
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: switch to uv
#122
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
=======================================
Coverage 95.42% 95.42%
=======================================
Files 7 7
Lines 284 284
=======================================
Hits 271 271
Misses 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughExpanded Windows clang-tools case in GitHub Actions to include version 20. Removed Sphinx Makefile and Windows batch script from docs. Moved sphinx-immaterial dependency from docs/requirements.txt into a new [dependency-groups].docs section in pyproject.toml. Changes
Sequence Diagram(s)sequenceDiagram
participant GA as GitHub Actions
participant Job as Test Matrix Job
participant Step as Check clang-tools (Windows)
GA->>Job: Run with matrix.version
Job->>Step: Determine tool paths
alt Windows and version in {15,16,18,20}
Step->>Step: Use Windows-specific clang-* executables
else Otherwise
Step->>Step: Use alternative/else branch
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes ✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
🔭 Outside diff range comments (2)
.github/workflows/python-test.yml (2)
107-117
: Inconsistent handling of Windows non-suffixed binaries; include 18 and 20 here as well.The “Show path of binaries” step only treats 15 and 16 as non-suffixed on Windows, but the later check step handles 15, 16, 18, 20. Align them to avoid false negatives.
Apply this diff:
- if [ "${{ matrix.version }}" = "15" -o "${{ matrix.version }}" = "16" ] && [ "${{ matrix.os }}" = "windows-latest" ]; then + if [ "${{ matrix.os }}" = "windows-latest" ] && echo "15 16 18 20" | grep -qw "${{ matrix.version }}"; then which clang-format which clang-tidy which clang-query which clang-apply-replacements elif [ "${{ matrix.os }}" = "ubuntu-latest" ]; then which "clang-format-${{ matrix.version }}" which "clang-tidy-${{ matrix.version }}" which "clang-query-${{ matrix.version }}" which "clang-apply-replacements-${{ matrix.version }}" fi
139-151
: macOS step never runs: matrix uses “macos-13” but condition checks “macos-latest”.As written, the Unix check won’t execute on macOS builds. Fix the condition to include macOS 13 (or use startsWith).
Apply this diff:
- - name: Check clang-tools on Unix - if: matrix.os == 'ubuntu-latest' || matrix.os == 'macos-latest' + - name: Check clang-tools on Unix + if: matrix.os == 'ubuntu-latest' || matrix.os == 'macos-13' run: | if [ "${{ matrix.version }}" = "12.0.1" -a "${{ matrix.os }}" = "ubuntu-latest" ]; then clang-format-12.0.1 --version clang-tidy-12.0.1 --version clang-query-12.0.1 --version clang-apply-replacements-12.0.1 --version else clang-format-${{ matrix.version }} --version clang-tidy-${{ matrix.version }} --version clang-query-${{ matrix.version }} --version clang-apply-replacements-${{ matrix.version }} --version fiAlternatively, make it future-proof:
- if: matrix.os == 'ubuntu-latest' || matrix.os == 'macos-latest' + if: matrix.os == 'ubuntu-latest' || startsWith(matrix.os, 'macos')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/python-test.yml
(1 hunks)docs/Makefile
(0 hunks)docs/make.bat
(0 hunks)docs/requirements.txt
(0 hunks)pyproject.toml
(1 hunks)
💤 Files with no reviewable changes (3)
- docs/requirements.txt
- docs/make.bat
- docs/Makefile
⏰ 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)
- GitHub Check: install (12.0.1, windows-latest)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
pyproject.toml (1)
95-98
: Verify that shared Sphinx workflow installs thedocs
group withuv
Our search did not uncover any
uv
commands or docs-group installs in this repo’s workflows, nor any lingeringdocs/requirements.txt
references. Since you’ve removeddocs/requirements.txt
and moved the Sphinx theme into the[dependency-groups]
:
- Ensure your local docs build (or the downstream reusable workflow in
cpp-linter/.github
) invokes one of:
uv sync -g docs
uv pip install -e . -g docs
- Confirm any
.github/workflows/*.yml
jobs that build docs include that step before runningsphinx-build
, sosphinx-immaterial
is actually installed..github/workflows/python-test.yml (1)
124-129
: Windows case extended to include LLVM 20 — LGTM.Adding 20 to the non-suffixed Windows binaries case matches the observed naming on those LLVM distributions.
Thanks for the test fix! |
I will remove the useless docs/requirements.txt after cpp-linter/.github#38 is merged.
Summary by CodeRabbit
Documentation
Chores