Code hygiene pass #90

Merged
rob merged 77 commits from refactor/code-hygiene into main 2026-09-28 13:13:44 +00:00
Collaborator

Collects the findings of a full code review into one branch: duplicated logic extracted, pass-throughs and trivial wrappers removed, redundant enum lookups dropped, and docstrings that only re-spell the code deleted.

Each item lands as its own PR into this branch. Merge into main once the list is done and the pipeline is green.

Collects the findings of a full code review into one branch: duplicated logic extracted, pass-throughs and trivial wrappers removed, redundant enum lookups dropped, and docstrings that only re-spell the code deleted. Each item lands as its own PR into this branch. Merge into main once the list is done and the pipeline is green.
Stop re-wrapping device classes already in their enum form
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 3m1s
Lint, type check and test / quality (pull_request) Failing after 3m11s
Lint, type check and test / release (pull_request) Has been skipped
6eabe3cafe
Merge pull request 'Simplify semantic-class lookups and drop no-op device class wraps' (#93) from refactor/direct-semantic-lookups into refactor/code-hygiene
Some checks failed
Lint, type check and test / quality (pull_request) Has been cancelled
Lint, type check and test / hassfest (pull_request) Has been cancelled
Lint, type check and test / release (pull_request) Has been cancelled
7dfb8765d5
Drop tools docstrings that only re-spell the code
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 19s
Lint, type check and test / quality (pull_request) Failing after 35s
Lint, type check and test / release (pull_request) Has been skipped
34a8911dc6
Merge pull request 'Drop tools docstrings that only re-spell the code' (#92) from refactor/tools-comment-prune into refactor/code-hygiene
Some checks failed
Lint, type check and test / quality (pull_request) Failing after 1m41s
Lint, type check and test / hassfest (pull_request) Successful in 1m42s
Lint, type check and test / release (pull_request) Has been skipped
427c37692d
Deletes the four call sites that caught and re-raised it, since it already carries a usable message.
update.py and services.py each carried their own copy of the same frozenset because of an import cycle. It belongs on the outcome itself.
Share the asking/request/expect pattern in commands.py
Some checks failed
Lint, type check and test / quality (pull_request) Failing after 2m19s
Lint, type check and test / hassfest (pull_request) Successful in 1m39s
Lint, type check and test / release (pull_request) Has been skipped
0808aad6d9
One _ask helper replaces the repeated triple in each command method, and async_switch computes its direction once.
Merge pull request 'Tidy transport, commands and firmware error handling' (#96) from refactor/transport-commands-hygiene into refactor/code-hygiene
Some checks failed
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
Lint, type check and test / hassfest (pull_request) Has been cancelled
7a14576b17
Share running_label between entity and updater link
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 16s
Lint, type check and test / quality (pull_request) Failing after 27s
Lint, type check and test / release (pull_request) Has been skipped
246a2a3218
Shares _session_response between update and flash for the BUSY/ACCEPTED
shapes, and parses the node address once per route instead of three
times. Also trims the stale comment above flash's lock note.
Name every updater logger after its own module
Some checks failed
Lint, type check and test / quality (pull_request) Failing after 1m22s
Lint, type check and test / hassfest (pull_request) Successful in 1m12s
Lint, type check and test / release (pull_request) Has been skipped
3aee0bb724
Merge pull request 'Deduplicate updater helpers and tidy logging' (#91) from refactor/updater-shared-helpers into refactor/code-hygiene
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Failing after 21s
Lint, type check and test / release (pull_request) Has been skipped
2dd8c41b06
Remove duplicated assignments from entity __init__ methods
Some checks failed
Lint, type check and test / quality (pull_request) Failing after 1m2s
Lint, type check and test / hassfest (pull_request) Successful in 1m35s
Lint, type check and test / release (pull_request) Has been skipped
c7f53b8a75
Merge pull request 'Dedupe discovery and entity boilerplate' (#95) from refactor/dedupe-discovery-entity into refactor/code-hygiene
Some checks failed
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / hassfest (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
aa0e1e7d17
Refresh the shared codec vectors
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 46s
Lint, type check and test / quality (pull_request) Successful in 2m8s
Lint, type check and test / release (pull_request) Has been skipped
7dbef8a0dc
Three SENSOR_SCALAR and SYS_ENDPOINT_META vectors now name 0x81 as their
source. The layout is unchanged; every pull request pipeline fails the
freshness check until this lands.
Merge pull request 'Refresh the shared codec vectors' (#97) from fix/refresh-vectors into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 45s
Lint, type check and test / quality (pull_request) Successful in 2m10s
Lint, type check and test / release (pull_request) Has been skipped
6a24ff339b
Inline the single-call _changed wrappers
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 19s
Lint, type check and test / quality (pull_request) Successful in 2m9s
Lint, type check and test / release (pull_request) Has been skipped
f76a319545
Merge pull request 'Lift a Deadlines helper, unify the (node, index) Key' (#94) from refactor/deadlines-and-key-cleanup into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 44s
Lint, type check and test / quality (pull_request) Successful in 2m23s
Lint, type check and test / release (pull_request) Has been skipped
06e7697379
Lift a shared known() lookup for the IntEnum fallback pattern
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 22s
Lint, type check and test / quality (pull_request) Successful in 2m0s
Lint, type check and test / release (pull_request) Has been skipped
7570f537dd
Merge pull request 'Lift a shared known() lookup for the IntEnum fallback pattern' (#99) from refactor/enum-lookup-helper into refactor/code-hygiene
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 10s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
cbc0f7d58a
Remove coordinator pass-through methods, call stores directly
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m46s
Lint, type check and test / release (pull_request) Has been skipped
49d73804ae
Merge pull request 'Remove coordinator pass-through methods, call stores directly' (#98) from refactor/coordinator-passthroughs into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 2m31s
Lint, type check and test / release (pull_request) Has been skipped
892839a095
Prune redundant docstrings in tools/frame_bench/
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m50s
Lint, type check and test / release (pull_request) Has been skipped
ee1cf0209f
Merge pull request 'Prune redundant docstrings in updater/ and tools/' (#100) from refactor/prune-redundant-docstrings into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 12s
Lint, type check and test / quality (pull_request) Successful in 1m52s
Lint, type check and test / release (pull_request) Has been skipped
d2ddc7de7c
Restore two spec-citing constant docstrings in session.py
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m54s
Lint, type check and test / release (pull_request) Has been skipped
3293634429
Merge pull request 'Restore two spec-citing constant docstrings in session.py' (#101) from fix/restore-spec-docstrings into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 33s
Lint, type check and test / quality (pull_request) Successful in 2m2s
Lint, type check and test / release (pull_request) Has been skipped
c55931b7a0
Prune redundant docstrings in custom_components/campervan
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m55s
Lint, type check and test / release (pull_request) Has been skipped
240b299d91
Merge pull request 'Prune redundant docstrings in custom_components/campervan' (#102) from chore/prune-restating-comments into refactor/code-hygiene
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
ae979feaaf
Restore three docstrings trimmed to fragments
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 22s
Lint, type check and test / quality (pull_request) Successful in 2m6s
Lint, type check and test / release (pull_request) Has been skipped
bb94a77904
Merge pull request 'Restore three docstrings trimmed to fragments' (#103) from fix/restore-trimmed-docstrings into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m44s
Lint, type check and test / release (pull_request) Has been skipped
a15b6c5a6f
Make module and class docstrings optional too
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 24s
Lint, type check and test / quality (pull_request) Successful in 1m56s
Lint, type check and test / release (pull_request) Has been skipped
4a2641bbee
Merge pull request 'Make module and class docstrings optional too' (#104) from chore/optional-docstrings into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 48s
Lint, type check and test / quality (pull_request) Successful in 2m18s
Lint, type check and test / release (pull_request) Has been skipped
0064c2c7f9
Define the control API port once, in protocol
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m49s
Lint, type check and test / release (pull_request) Has been skipped
340ecfbe28
Merge pull request 'Fix three code-smell comments in updater and firmware port' (#105) from chore/updater-status-and-port into refactor/code-hygiene
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 13s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
e98999bf72
Extract ready_to_name for the two follow_the_inventory closures
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 59s
Lint, type check and test / quality (pull_request) Successful in 2m20s
Lint, type check and test / release (pull_request) Has been skipped
e1a2ffed49
Merge pull request 'Fix six spots where a comment was standing in for clearer code' (#106) from chore/fix-comment-smells into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m50s
Lint, type check and test / release (pull_request) Has been skipped
e2c2f0ad6a
Sort cached images in two passes again
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m52s
Lint, type check and test / release (pull_request) Has been skipped
519e457d86
Negating the version tuple does not reverse the rule that a shorter prefix
sorts first, so a version with fewer segments came out newest. Two stable
passes are the documented idiom for mixed directions.
Merge pull request 'Sort cached images in two passes again' (#107) from fix/image-sort-prefix into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 38s
Lint, type check and test / quality (pull_request) Successful in 2m4s
Lint, type check and test / release (pull_request) Has been skipped
8ebfe264bc
Prune redundant comments in tools/
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 17s
Lint, type check and test / quality (pull_request) Successful in 1m54s
Lint, type check and test / release (pull_request) Has been skipped
e3b50edacf
Merge pull request 'Prune redundant comments in protocol/ and tools/' (#109) from chore/prune-protocol-tools-comments into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m46s
Lint, type check and test / release (pull_request) Has been skipped
67c103901c
Restore the trap and spec-cite comments the prune cut
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m50s
Lint, type check and test / release (pull_request) Has been skipped
aed33fc535
Merge pull request 'Restore the trap and spec-cite comments the prune cut' (#110) from fix/restore-trap-comments into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m58s
Lint, type check and test / release (pull_request) Has been skipped
ea4f2d34d6
Prune comments and docstrings that only restate the code in updater/ (#108)
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 27s
Lint, type check and test / quality (pull_request) Successful in 2m0s
Lint, type check and test / release (pull_request) Has been skipped
b89a80188d
Prune restating comments in the firmware module
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 7s
Lint, type check and test / quality (pull_request) Successful in 1m47s
Lint, type check and test / release (pull_request) Has been skipped
34b99d1dcb
Say why a setting skips the sentinel check
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m53s
Lint, type check and test / release (pull_request) Has been skipped
7892b21c48
Merge pull request 'Prune restating comments in custom_components/campervan root modules' (#111) from chore/prune-campervan-root-comments into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 25s
Lint, type check and test / quality (pull_request) Successful in 2m10s
Lint, type check and test / release (pull_request) Has been skipped
677f749692
Restore the one-line traps the integration prune cut
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m45s
Lint, type check and test / release (pull_request) Has been skipped
5480ea89ca
Merge pull request 'Restore the one-line traps the integration prune cut' (#112) from fix/restore-editor-traps into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m46s
Lint, type check and test / release (pull_request) Has been skipped
c9c023b418
rob requested changes 2026-09-28 11:26:26 +00:00
@ -482,0 +377,4 @@
self._finish()
def _finish(self) -> None:
# Cleared before the notify: a listener acting on `settled` must see
Owner

This is hard to read. If it really needs to be here, simplify the prose

This is hard to read. If it really needs to be here, simplify the prose
@ -66,2 +57,4 @@
TOOK = frozenset({"updated", "returned"})
"""The updater's words for a session that left the node running something.
Owner

More poor English

More poor English
@ -10,2 +10,4 @@
from .units import Sentinel
CONTROL_API_PORT = 8787
"""Clear of Home Assistant's 8123, which shares the host's network namespace."""
Owner

Don't need this comment

Don't need this comment
@ -54,3 +51,2 @@
when: str = ""
# Where set, only these values of field `when` put this one on the wire.
# What to send, never what to expect: a reader goes by the length it got.
# Only these values of `when` put this field on the wire. Encoding only: a
Owner

WTF does this even mean?

WTF does this even mean?
@ -19,3 +18,1 @@
Never raised here: only a consumer knows which fields it cannot act without.
"""
"""Never raised here: only a consumer knows which fields it cannot act without."""
Owner

Surely this wants to be a pass like PayloadTooLongError?

Surely this wants to be a `pass` like `PayloadTooLongError`?
@ -0,0 +1,22 @@
"""Looking a raw int up in an `IntEnum`, the bus's one recurring shape.
Owner

Horrible prose in this comment. Ideally remove it, but at least sort it out

Horrible prose in this comment. Ideally remove it, but at least sort it out
@ -4,3 +1,1 @@
one dropped looks like a fault in the van. The bridge's own counts of the bus
land here too, sharing nothing with the rest but the platform.
"""
"""Everything the van measures, and everything it says nothing useful about."""
Owner

What does this even mean?!

What does this even mean?!
@ -54,3 +43,1 @@
A total stays true with the interface down — it is what happened before it
went — and that is exactly when someone is reading it.
"""
"""Stays available with the interface down: it is what happened before it went."""
Owner

This makes no sense

This makes no sense
@ -1,4 +1,7 @@
"""What the Pi can see of the bus, counted rather than interpreted."""
"""What the Pi can see of the bus, counted rather than interpreted.
Owner

This is a shit comment. Rewrite it in simpler English

This is a shit comment. Rewrite it in simpler English
@ -1,4 +1,7 @@
"""What the Pi can see of the bus, counted rather than interpreted."""
"""What the Pi can see of the bus, counted rather than interpreted.
No payload is decoded here: a second decoder is a second chance to drift.
Owner

Why was this added? It should be clear reading the code that this isn't decoding

Why was this added? It should be clear reading the code that this isn't decoding
@ -168,3 +147,1 @@
Missing is what has spoken this run and since gone quiet: nothing else
says what the van ought to have, and a board taken out is not missing.
"""
"""Missing spoke this run and went quiet since; a board taken out has not."""
Owner

Unintelligible

Unintelligible
Rewrite the terse comments in plain English
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 21s
Lint, type check and test / quality (pull_request) Successful in 2m8s
Lint, type check and test / release (pull_request) Has been skipped
656f488c28
Review of #90 found the comments the hygiene pass wrote hard to read.
Rewritten to say the thing directly, or deleted where the code already
says it. Ruff's D205 and D401 are turned off too: a docstring that states
a reason is not a summary line or a command.
Merge pull request 'Rewrite the terse comments in plain English' (#113) from fix/plain-english-comments into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 47s
Lint, type check and test / quality (pull_request) Successful in 2m18s
Lint, type check and test / release (pull_request) Has been skipped
842cd83c6b
Rewrite the remaining terse comments in plain English
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m53s
Lint, type check and test / release (pull_request) Has been skipped
68389ccb6f
A full pass over every comment and docstring left after the hygiene work.
Each one now says the thing directly, or is gone where the code already
says it.
Merge pull request 'Rewrite the remaining terse comments in plain English' (#114) from fix/plain-english-sweep into refactor/code-hygiene
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 24s
Lint, type check and test / quality (pull_request) Successful in 1m57s
Lint, type check and test / release (pull_request) Has been skipped
1d6ebe200c
rob changed title from WIP: Code hygiene pass to Code hygiene pass 2026-09-28 13:13:39 +00:00
rob merged commit 991242d93a into main 2026-09-28 13:13:44 +00:00
rob deleted branch refactor/code-hygiene 2026-09-28 13:13:44 +00:00
Sign in to join this conversation.
No reviewers
rob
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!90
No description provided.