Chores (mainly migrating to prek) #845
Conversation
CoMPaTech
added
enhancement
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughMigrate CI and developer tooling from pre-commit to prek, replace node-based markdownlint with pymarkdown, remove Biome formatter and related hooks, update GitHub Actions inputs/versions and cache version, and adjust scripts, requirements, and linting configs accordingly. Changes
Sequence Diagram(s)(omitted — changes are configuration/tooling migrations without new multi-component runtime control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (1)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/verify.yml:
- Around line 6-9: The workflow fails to define PREK_HOME while referencing `${{
env.PREK_HOME }}`; add a PREK_HOME entry to the top-level env block (alongside
CACHE_VERSION, DEFAULT_PYTHON, VENV) so the variable is populated for all steps
— e.g., set PREK_HOME to the intended workspace/root path used by the repo (such
as the repo root or a subdirectory), or to `${{ github.workspace }}` if
appropriate, ensuring all steps that reference PREK_HOME receive a non-empty
value.
- Around line 116-119: The workflow step labeled "Lint markdown files" is
invoking the wrong pre-commit hook id; change the hook id from "markdownlint" to
"pymarkdown" so it matches the hook defined in .pre-commit-config.yaml; update
the run command that calls prek to use --hook-stage manual pymarkdown (replacing
markdownlint) to ensure the correct hook runs.
In @.pymarkdown:
- Line 1: Rename the unrecognized configuration file `.pymarkdown` to a
supported pymarkdown config filename (e.g., `pymarkdown.json`,
`pymarkdown.yaml`, or `pymarkdown.toml`) and move the existing JSON content `{
"plugins": { "md013": { "enabled": false } } }` into that new file so pymarkdown
can detect and load the configuration; if you choose YAML or TOML, convert the
JSON to the appropriate syntax.
🧹 Nitpick comments (4)
scripts/manual_fixtures.py (1)
33-33: Consider using context managers for file operations.Files are opened without context managers (
withstatement) and never explicitly closed. While the OS will clean up when the script exits, using context managers is a best practice that ensures proper resource cleanup.♻️ Example refactor for line 33
-io = open(basefile) -base = json.load(io) +with open(basefile) as io: + base = json.load(io)Apply similar changes to lines 51, 69, and 207.
Also applies to: 51-51, 69-69, 207-207
CHANGELOG.md (1)
6-9: LGTM! Minor wording suggestion.The changelog entries clearly document the tooling changes. Consider changing "pythonesk" to "Python-based" for clarity, as "pythonesk" is non-standard spelling (the conventional form would be "Pythonesque", but "Python-based" is more commonly used in technical contexts).
scripts/setup_test.sh (1)
24-26: Comment still references "pre-commit".The comment on line 24 mentions "pre-commit" but the tool has been migrated to
prek. Consider updating the comment for consistency.Suggested fix
-# Install prek hook unless running from within pre-commit +# Install prek hook unless running from within prek if [ "$#" -eq 0 ]; then prek install --install-hooks fiscripts/setup.sh (1)
22-23: Stale comment references old tool.The comment on line 22 still says "Install pre-commit hook" but the command now uses
prek. Update the comment for consistency with the migration.Suggested fix
-# Install pre-commit hook +# Install prek hook prek install --install-hooks
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/verify.yml.markdownlint.yaml.pre-commit-config.yaml.pymarkdownCHANGELOG.mdbiome.jsonplugwise/legacy/smile.pyplugwise/smile.pyrequirements_commit.txtscripts/manual_fixtures.pyscripts/setup.shscripts/setup_test.shscripts/tests_and_coverage.sh
💤 Files with no reviewable changes (2)
- biome.json
- .markdownlint.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-25T12:02:58.781Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 835
File: plugwise/helper.py:837-841
Timestamp: 2025-12-25T12:02:58.781Z
Learning: In the Python Plugwise library, zones are not separate appliances in gw_entities. When a zone is added to self.gw_entities, the location_id is used as the entity_id. In code reviews, validate that zone checks rely on location_id being present in _existing_zones and that lookups use self.gw_entities[location_id]. Do not treat zones as independent appliances.
Applied to files:
plugwise/smile.pyplugwise/legacy/smile.py
📚 Learning: 2024-11-28T19:54:45.351Z
Learnt from: CoMPaTech
Repo: plugwise/python-plugwise PR: 656
File: .pre-commit-config.yaml:9-9
Timestamp: 2024-11-28T19:54:45.351Z
Learning: The project's dependencies and pre-commit hooks (e.g., ruff at v0.8.0, bandit at 1.8.0, pyupgrade at v3.19.0) are confirmed to be compatible with Python 3.13.
Applied to files:
requirements_commit.txt
🪛 LanguageTool
CHANGELOG.md
[grammar] ~9-~9: Ensure spelling is correct
Context: ...ased markdownlint with pythonesk library ## v1.11.2 - Add/update model-data for Jip...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (7)
plugwise/legacy/smile.py (1)
222-231: LGTM!Typo correction in the docstring looks good. This aligns with the PR objective of fixing typos reported by codespell.
plugwise/smile.py (1)
401-410: LGTM!Consistent typo correction matching the fix in
plugwise/legacy/smile.py. Documentation improvement looks good.scripts/manual_fixtures.py (1)
39-44: LGTM! Formatting improvement aligns with tooling migration.The multi-line formatting improves readability and is consistent with the PR's migration to prek/prettier.
requirements_commit.txt (1)
8-8: LGTM!The dependency change from
pre-committoprekaligns with the PR's migration objective and is consistent with the script and workflow updates throughout the repository.scripts/tests_and_coverage.sh (1)
48-49: LGTM!The migration from
pre-committoprek install --install-hooksis consistent with the other script changes in this PR. The placement alongside dependency installation is appropriate..github/workflows/verify.yml (1)
47-55: Migration to prek looks good, but verifyPREK_HOMEdefinition.The step naming and input changes from
precommit-hometoprek-homeare consistent with the migration. Ensure thePREK_HOMEenvironment variable is defined (see earlier comment)..pre-commit-config.yaml (1)
114-118: Configuration is correct; args are optional.The pymarkdown pre-commit hook works without additional args—the
scancommand runs by default. The args configuration is optional and only needed if you require custom settings like a non-default config file path.Likely an incorrect or invalid review comment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #845 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3443 3443
=========================================
Hits 3443 3443 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/verify.yml:
- Around line 6-9: The workflow sets no PREK_HOME in the env block but passes
the input prek-home: ${{ env.PREK_HOME }} into multiple actions (e.g.,
prepare-python-and-code@v2 and restore-venv@v2), which causes an empty string to
override the actions' defaults; remove the prek-home: ${{ env.PREK_HOME }} input
from all action usages (all seven instances where prek-home is present) so the
actions use their built-in PREK_HOME behavior instead.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/verify.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T16:49:05.347Z
Learnt from: CoMPaTech
Repo: plugwise/python-plugwise PR: 845
File: .github/workflows/verify.yml:6-9
Timestamp: 2026-01-12T16:49:05.347Z
Learning: In repositories using plugwise/python-plugwise conventions, PREK_HOME is already set by default by actions such as prepare-python-and-codev2 and restore-venvv2. Do not redefine PREK_HOME in workflow files that use these actions. This guidance applies to workflow files under .github/workflows in the repository (and can be extended to similar workflows that rely on these actions).
Applied to files:
.github/workflows/verify.yml
🔇 Additional comments (4)
.github/workflows/verify.yml (4)
39-39: Cache key still references.pre-commit-config.yaml.The cache key hash includes
.pre-commit-config.yaml. If prek uses a different configuration file, this should be updated to ensure cache invalidation when the prek config changes.If
.pre-commit-config.yamlis retained for backwards compatibility (as mentioned in the PR description), this is fine. Otherwise, consider updating to reference the prek configuration file.
47-55: Migration to v2 action looks consistent.The step name and action version upgrade from
v1tov2align with the prek migration. Theprek-homeinput rename is appropriate, pending resolution of the undefinedPREK_HOMEissue noted above.
115-119: prek command migration looks correct.The replacement of
pre-commit runwithprek runand the migration frommarkdownlinttopymarkdownalign with the PR objectives. The command flags remain consistent with the previous pre-commit invocation style.
68-75: Consistent action version upgrades across all jobs.All
restore-venvaction references have been consistently upgraded fromv1tov2, and the input has been renamed fromprecommit-hometoprek-homethroughout. This maintains consistency across the workflow.Also applies to: 104-111, 139-147, 172-179, 217-224, 299-306
There was a problem hiding this comment.
Pull request overview
This pull request migrates the project from pre-commit to prek while maintaining backwards compatibility, simplifies the tooling by removing biome in favor of the reintroduced prettier, and migrates from node-based markdown linting to Python-based pymarkdown. The PR also fixes two spelling errors in code comments.
Changes:
- Migrated from pre-commit to prek across scripts, requirements, and GitHub workflows
- Removed biome formatter and related configuration
- Switched from markdownlint (node-based) to pymarkdown (Python-based)
- Fixed spelling typos in docstrings ("succesful" → "successful")
- Updated GitHub Actions to v2 and bumped cache version
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/tests_and_coverage.sh | Removed biome formatting calls and updated to use prek install |
| scripts/setup_test.sh | Removed biome setup logic and updated to use prek install |
| scripts/setup.sh | Updated pre-commit install to prek install |
| scripts/manual_fixtures.py | Reformatted long dictionary access lines for better readability |
| requirements_commit.txt | Replaced pre-commit with prek dependency |
| plugwise/smile.py | Fixed typo: "succesful" → "successful" in docstring |
| plugwise/legacy/smile.py | Fixed typo: "succesful" → "successful" in docstring |
| biome.json | Removed biome configuration file |
| CHANGELOG.md | Added changelog entry for these changes |
| .pymarkdown | Added pymarkdown configuration disabling line length rule |
| .pre-commit-config.yaml | Removed biome hook and setup_test hook, replaced markdownlint with pymarkdown |
| .markdownlint.yaml | Removed old markdownlint configuration |
| .github/workflows/verify.yml | Updated to use GitHub Actions v2, prek instead of pre-commit, removed biome lint step, bumped cache version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
coderabbitai
bot
added
the
github_actions
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
uv pip install prek; prek install -fto migrate andprek run --all-filesto check your bearingsbiomejsapproachSummary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.