Cut the frame bench and vectors guard comments to their reason #72
Loading…
Reference in a new issue
No description provided.
Delete branch "docs/frame-bench-comments"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A comment pass over what this branch added to
frame_bench, the vectorsrefresh script and their tests. Narration and restatement are gone; what is
left is rationale, traps and spec citations, each cut to a line where it fits.
The one behaviour-adjacent change is in
codec.py:_shortest_lengthhad afive-line docstring explaining a dense loop, so the loop now says it. The two
cases that end a frame early moved into
_ends_the_frame_at, and the docstringis one line. Same lengths out, covered by the existing bench tests.
tools/refresh-vectorskeeps its guard and its reason — a merged firmwarebranch is kept, so its vectors freeze the day it lands — just in three lines
rather than five. No change to what the guard does.
Test docstrings that only restate the test name could not simply be deleted:
ruffhasDselected with no per-file ignores, so D103 requires one on everypublic test function. Those were rewritten to carry a reason instead.
Comment and docstring lines across the ten files in scope: 348 before, 314
after. Of those, the lines this branch introduced went from 92 to 58.
Lint, format, mypy and the full suite are green.
Five wording points, inline. Nothing behavioural.
@ -49,3 +48,2 @@run: |# Logged because the guard below turns on it, and a runner that# leaves base_ref empty would skip it with nothing to show for it.# A runner that leaves base_ref empty skips the guard silently.The comment no longer says why the
echobelow it exists, so it reads as a stray fact above a line someone could take for noise and delete. Suggest:# Logged because the guard turns on it: a runner leaving base_ref empty skips it silently.@ -107,3 +104,1 @@One side sending eight bytes where the other expects two is the drift thiswhole file exists to catch, and no byte of the payload gives it away.""""""One side sending eight bytes where the other expects two, no byte wrong.""""no byte wrong" is cryptic — the point was that no byte of the payload gives the drift away, which is why comparing frames would not catch it. Suggest:
Both sides must agree: eight bytes where the other expects two, with no byte of the payload wrong.@ -135,3 +135,3 @@def _hex_bytes(raw: object, width: int, trailing: bool = False) -> bytes:"""Read a run of bytes typed as hex, padded out where it may end the frame.""""""Read a run of bytes typed as hex, short only where it may end the frame."""_hex_bytesalways returns exactlywidthbytes (value.ljust(width, b"\0")), so "short only where it may end the frame" reads as a short return that never happens. It is the accepted input that may be short. The old wording at least named the padding.@ -435,2 +435,4 @@def _ends_the_frame_at(spec: FrameField, typed: object) -> int | None:"""Give where a field's own size ends the frame: text, and a last `BULK_DATA`."""Private, so D103 does not force a docstring, and this one restates the two
isinstancebranches beneath it. The only thing it adds — that trailing means a lastBULK_DATA— is already onRawField.trailing. Delete it.@ -73,3 +71,1 @@The frame then stops before it, because section 8 has a sender use theshortest length that carries what it means.""""""Only these values of field `when` carry it; the frame stops there. Section 8."""The meaning flipped. "the frame stops there" attaches to the case where the field is carried; the original said the frame stops before it when it is not. Suggest:
Only these values of fieldwhenput it on the wire; otherwise the frame stops before it. Section 8.All five taken. Nothing further.