Maintenance chores #754
Maintenance chores #754
Conversation
CoMPaTech
added
enhancement
WalkthroughThe updates refactor and simplify the project's setup and test scripts, removing reliance on an external virtual environment helper script and the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SetupScript
participant Venv
participant Tools
User->>SetupScript: Run setup.sh or setup_test.sh
SetupScript->>Venv: Check if venv active
alt Not active
SetupScript->>Venv: Create venv (with uv or python3)
Venv-->>SetupScript: venv activated
end
SetupScript->>Tools: Install dependencies (uv pip install)
SetupScript->>Tools: Install pre-commit hooks
User-->>SetupScript: Ready to use environment
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
scripts/tests_and_coverage.sh (1)
53-53:⚠️ Potential issueEnsure
uvis available before use
You invokeuv pip install …without installing or verifyinguv. Either installuvin this script or fallback to plainpip install.
🧹 Nitpick comments (13)
scripts/complexity.sh (1)
6-18: Enhance ShellCheck compliance for dynamic sourcing
The new virtualenv detection is robust. To satisfy ShellCheck SC1090, consider adding a directive above the.statement, e.g.:# shellcheck source=/dev/null . "${VIRTUAL_ENV}/bin/activate"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 14-14: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
scripts/setup.sh (2)
2-3: Remove or use unusedmy_pathvariable
my_pathis set but never referenced in this script. Either drop the assignment or employ it consistently when creating/activating the venv.
7-14: Simplify virtualenv existence check
Use[ -z "$VIRTUAL_ENV" ]instead of[ ! -n "$VIRTUAL_ENV" ]for clarity, and consider checkingpython3 -m venvavailability before relying onuv.scripts/tests_and_coverage.sh (2)
5-7: Remove unusedOSTYPEdefaulting
OSTYPE="${OSTYPE-}"isn’t referenced later. If needed by external scripts, document it; otherwise delete these lines.
17-30: Add ShellCheck directive for dynamic sourcing
Sourcing non-constant paths (. "${my_path}/${venv}/bin/activate") triggers SC1090. Suppress or annotate it:# shellcheck disable=SC1090 . "${my_path}/${venv}/bin/activate"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 25-25: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
scripts/run-in-env.sh (2)
5-7: Eliminate unusedOSTYPEassignment
You setOSTYPE="${OSTYPE-}"but never use it. Remove these lines or document their purpose.
17-29: Suppress ShellCheck warning for dynamic sources
Add a directive to silence SC1090 for the non-constantsourcecalls:# shellcheck disable=SC1090 . "${VIRTUAL_ENV}/bin/activate"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 25-25: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
scripts/setup_test.sh (6)
7-14: Use-zfor empty checks
if [ ! -n "$VIRTUAL_ENV" ]is equivalent but less idiomatic thanif [ -z "$VIRTUAL_ENV" ]. Update accordingly:-if [ ! -n "$VIRTUAL_ENV" ]; then +if [ -z "$VIRTUAL_ENV" ]; then
16-18: Installuvinto the activated venv
You runpython3 -m pip install uvbefore confirming the venv is activated. Either activate first or useuv pip install uvpost-activation.
20-20: Createtmpunder repo root
mkdir -p ./tmpis relative to CWD. Prefer the repository root variable:mkdir -p "${my_path}/tmp"
23-23: Split longuv pip installfor readability
This command is very lengthy and could hit shell limits. Consider line breaks with\for clarity.
27-30: Improve Biome CLI architecture detection
Currentgreponuname -amay misclassify OS/arch combos (e.g., x86_64 macOS). Useuname -sanduname -mfor precise checks.
40-42: Also install pre-commit hooks
You runpre-commit installonly when no args are passed. To align with other scripts, consider addingpre-commit install-hooks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.pre-commit-config.yaml(1 hunks)pyproject.toml(2 hunks)scripts/complexity.sh(1 hunks)scripts/manual_fixtures.py(8 hunks)scripts/python-venv.sh(0 hunks)scripts/run-in-env.sh(1 hunks)scripts/setup.sh(1 hunks)scripts/setup_test.sh(1 hunks)scripts/tests_and_coverage.sh(1 hunks)setup.cfg(0 hunks)
💤 Files with no reviewable changes (2)
- setup.cfg
- scripts/python-venv.sh
🧰 Additional context used
🪛 Shellcheck (0.10.0)
scripts/complexity.sh
[warning] 14-14: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
scripts/tests_and_coverage.sh
[warning] 25-25: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
scripts/run-in-env.sh
[warning] 25-25: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
scripts/setup.sh
[warning] 5-5: my_path appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (11)
pyproject.toml (2)
26-26: Bump minimum Python version to 3.13
Updatingrequires-pythonto>=3.13aligns with the project’s decision to drop 3.12 support. Ensure CI and all Docker/base images are updated accordingly.
54-54: Clean up obsolete Pylint disable comment
Removing the"implicit-str-concat"disable (ISC001) is fine if no occurrences remain. This keeps lint settings accurate.scripts/manual_fixtures.py (1)
39-44: Formatting tweaks only—logic unchanged
All adjusted dictionary key accesses and assignments have been reformatted for readability without altering behavior. The fixtures updates remain functionally equivalent.Also applies to: 80-86, 100-100, 103-107, 112-116, 125-130, 142-146, 147-147, 151-151, 155-159, 160-161, 164-168, 171-176, 178-182, 205-207, 208-216, 243-243, 297-299
.pre-commit-config.yaml (1)
36-42: Refined Codespell hook configuration
The rename to “Check Code Spelling” and addedargs(ignore-words, skip patterns, quiet level) plusexcludeforuserdata/andfixtures/consolidate spell-check settings cleanly in one place.scripts/complexity.sh (1)
20-23: Cyclomatic complexity invocation looks good
Unconditionalradon ccexecution after environment activation ensures metrics are always generated.scripts/setup.sh (2)
16-18: Installuvvia pip when missing
Unconditionally installinguvensuresuv venvis available for subsequent runs—this is solid.
21-24: Streamlined dependency and hook setup
Usinguv pip install ...andpre-commit installunconditionally simplifies maintenance.scripts/tests_and_coverage.sh (1)
12-15: pyenv activation is correct
The check for.python-versionand exportingPYENV_VERSIONis implemented properly.scripts/run-in-env.sh (3)
1-1: POSIXshcompliance looks good
Changing the shebang toshis acceptable and no bash-only features remain.
12-15: pyenv block approved
Detecting and exportingPYENV_VERSIONfrom.python-versionis implemented correctly.
31-31: Correct use ofexec
exec "$@"cleanly replaces the shell with the desired command.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/complexity.sh (2)
11-11: Nitpick: Duplicatemy_pathassignment
You already setmy_pathat line 4; the second assignment at line 11 is redundant and can be removed.
22-25: Ensure radon runs from the repo root
If the script is invoked from outside the project root,$(pwd)may not point to the expected location. Consider:+cd "$my_path" -PYTHONPATH=$(pwd) radon cc plugwise/ tests/ -s -nc --no-assert +PYTHONPATH="$my_path" radon cc "$my_path"/plugwise/ "$my_path"/tests/ -s -nc --no-assertscripts/setup.sh (1)
5-5: Use safer expansion forVIRTUAL_ENV
Adopt${VIRTUAL_ENV-}to guard against unset variables, aligning with other scripts:-if [ -z "$VIRTUAL_ENV" ]; then +if [ -z "${VIRTUAL_ENV-}" ]; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.pre-commit-config.yaml(2 hunks)scripts/complexity.sh(1 hunks)scripts/run-in-env.sh(1 hunks)scripts/setup.sh(1 hunks)scripts/setup_test.sh(1 hunks)scripts/tests_and_coverage.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- .pre-commit-config.yaml
- scripts/tests_and_coverage.sh
- scripts/run-in-env.sh
- scripts/setup_test.sh
🔇 Additional comments (4)
scripts/complexity.sh (1)
6-8: Robust virtualenv activation
Checking${VIRTUAL_ENV-}and verifying the activate script ensures reliable activation across environments.scripts/setup.sh (3)
2-2: Provenance comment is fine
14-17: Installing theuvutility
This block ensuresuvis present before use. It's implemented correctly.
19-23: Dependencies and pre-commit hook setup
Theuv pip installandpre-commit installcommands look good. Confirmuvis on$PATHpost-installation; considerhash -rif necessary.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #754 +/- ##
=========================================
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:
|
|
@coderabbitai please update the summary |
|
Tip For best results, initiate chat on the files or code changes. @CoMPaTech Here’s the refreshed high-level summary of PR #754 after the latest commits. Updated high-level summary • Project metadata: • CHANGELOG Everything else in the PR (already reviewed earlier) stays the same: pre-commit now owns Codespell config, several helper scripts were refactored to locate/activate a venv on the fly, obsolete No functional code of the Plugwise library itself is touched; changes are maintenance-oriented (tooling, CI, metadata). |
|
Now for the though part - should we release it as 175 or not (seeing the PR in beta) |
|
1.7.5 is fine for the backend. But I would suggest to NOT bump the pw-beta version. |
Summary by CodeRabbit