-
Notifications
You must be signed in to change notification settings - Fork 8
Rework/backport changes between USB/network #759
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
WalkthroughA custom GitHub Action for restoring or creating Python virtual environments and pre-commit caches was introduced, centralizing and simplifying cache management in CI. The workflow was refactored to use this action, updating job dependencies and removing redundant cache logic. Additional minor updates include a new changelog entry, a new test dependency, and improved architecture detection in a setup script. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant CacheJob
participant Job as CI Job (e.g., prepare, ruff, pytest)
participant RestoreVenv as restore-venv Action
Workflow->>CacheJob: Start cache job
CacheJob-->>Workflow: Output cache-key
loop For each CI job
Workflow->>Job: Start job (needs: cache, prepare, etc.)
Job->>RestoreVenv: Invoke restore-venv with cache-key and params
alt Cache hit
RestoreVenv-->>Job: Restore venv and pre-commit cache
else Cache miss
RestoreVenv-->>Job: Create venv, install dependencies, (optionally) fail or save cache
end
Job-->>Workflow: Continue job steps
end
Suggested labels
Suggested reviewers
Poem
✨ 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. 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 (
|
|
|
@coderabbitai summary |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@CoMPaTech I'll gather the information about this PR and provide both a summary and full review. 🏁 Script executed: Length of output: 931 🏁 Script executed: Length of output: 80517 @CoMPaTech Here's the summary and full review of your PR: SummaryPR #759: Rework/backport changes between USB/network This PR implements significant CI/CD infrastructure improvements by backporting changes from python-plugwise-usb#264. The main focus is optimizing the GitHub Actions workflow through better caching strategies and code deduplication. Key Changes:
Impact: Reduces code duplication, improves build efficiency, and makes the CI/CD pipeline more maintainable. Full Review✅
|
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: 2
🧹 Nitpick comments (4)
.github/actions/restore-venv/action.yml (1)
16-16: Fix typo in default input description
The comment# DefauLt fail if not availablehas an inconsistent capital “L.”Apply:
- default: "true" # DefauLt fail if not available + default: "true" # Default to failing if cache restore missesCHANGELOG.md (1)
3-6: Unify link syntax for consistency
Linking to external PRs in other repos should follow[#264]convention for clarity.-## Ongoing - -- Maintenance chores ... [USB: 264](https://github.com/plugwise/python-plugwise-usb/pull/264) after porting our progress using [USB: 263](...) +## Ongoing + +- Maintenance chores ... [#264](https://github.com/plugwise/python-plugwise-usb/pull/264) after porting our progress using [#263](...)scripts/setup_test.sh (1)
28-33: Enhance cross-platform architecture detection
Mappingaarch64|arm64directly todarwin-arm64fails for Linux ARM hosts. Consider detecting OS viauname -sand then matchinguname -m, e.g.:os=$(uname -s) arch=$(uname -m) case "$os-$arch" in Darwin-arm64) use_arch="darwin-arm64" ;; Linux-aarch64) use_arch="linux-arm64" ;; Linux-x86_64) use_arch="linux-x64" ;; *) echo "Unsupported OS/arch: $os-$arch"; exit 2 ;; esac.github/workflows/verify.yml (1)
64-64: Remove trailing spaces
Several lines have trailing whitespace, which can lead to YAML lint errors. Please strip the trailing spaces on lines 64, 143, 182, 233, and 310.Also applies to: 143-143, 182-182, 233-233, 310-310
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/restore-venv/action.yml(1 hunks).github/workflows/verify.yml(16 hunks)CHANGELOG.md(1 hunks)requirements_test.txt(1 hunks)scripts/setup_test.sh(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/verify.yml
[error] 64-64: trailing spaces
(trailing-spaces)
[error] 143-143: trailing spaces
(trailing-spaces)
[error] 182-182: trailing spaces
(trailing-spaces)
[error] 233-233: trailing spaces
(trailing-spaces)
[error] 310-310: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
requirements_test.txt (1)
8-8: Add code coverage plugin for pytest
Includingpytest-covenabling coverage reporting in CI aligns with the updated workflows. Ensure that thepytest --covcommands pick up this plugin in the relevant jobs..github/workflows/verify.yml (4)
7-11: Centralize CI environment variables
DefiningCACHE_VERSION,PRE_COMMIT_HOME, andVENVat the top promotes consistency across all jobs.
20-29: Compute a unified cache key once
Calculating the cache key in a dedicated job based on OS, Python version, and key files reduces duplication and ensures cache coherence.
43-60: Use custom restore-venv action for environment setup
Replacing scatteredactions/cachesteps with therestore-venvcomposite action in thepreparejob streamlines cache restore and creation.
140-147: Matrix-based pytest cache key
Appending the matrix Python version to the cache key (-pytest-matrix-${{ matrix.python-version }}) prevents collisions across matrix runs.
bouwew
left a 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.
LGTM!
|
Still need to address the biome issues discovered in usb |
|
Re-running CI |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3410 3410
=========================================
Hits 3410 3410 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|



Following plugwise/python-plugwise-usb#264
Summary by CodeRabbit
Chores
Tests
Bug Fixes