Chores (mainly migrating to prek) #845
Conversation
CoMPaTech
added
enhancement
WalkthroughMigrate CI/hooks from pre-commit to prek, replace node-based markdownlint with jackdewinter/pymarkdown, remove Biome formatter, update scripts and requirements, and bump GitHub Actions inputs/versions and cache version across workflows and related files. 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 reviewers
Poem
🚥 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)
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
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.