Cleanup of five non-blocking findings from the independent re-review of #6. Every one was a wrong claim
in a comment, not wrong code. The change touches only comments and one
constant's name in cancel_test.go, plus a TODO.md entry.
The index-load "three consultations" figure was wrong (measured 6-9).
The walkClock comment now states the property the tests rely on —
the load's Done cost is bounded and independent of record count —
rather than a magic number.
Three tests lack the two-second bound the suite's framing implied;
they catch their regression only as the test binary's timeout. The poolUnwind comment no longer claims the bound is universal, and each
of the three now says it hangs to the binary timeout on regression.
The hashWorker results-send abandon branch is reachable from the
production scan path (stop() cancels before it drains), not dead
code, and is covered by TestScanHashWriteFailureUnwindsPool. A
comment now records that all cancellation branches are covered.
walkCancelInFlightDirs held a file count (80 files from 4 in-flight
directories); renamed to walkCancelInFlightFiles, same value.
Added a top-of-file comment recording the deliberate departure from
the one-test-file-per-source-file convention.
Disclosure (judgement call): the definition of done asks to re-verify
each test by the reviewer's mutation method. Because the change alters
only comments and one test-local constant's name, every test's compiled
behaviour is identical and no regression property changes, so I did not
repeat the mutation matrix. make check is green.
Model: opus-4-8
Cleanup of five non-blocking findings from the independent re-review of
https://git.eeqj.de/sneak/sfdupes/issues/6. Every one was a wrong claim
in a comment, not wrong code. The change touches only comments and one
constant's name in `cancel_test.go`, plus a `TODO.md` entry.
- The index-load "three consultations" figure was wrong (measured 6-9).
The `walkClock` comment now states the property the tests rely on —
the load's Done cost is bounded and independent of record count —
rather than a magic number.
- Three tests lack the two-second bound the suite's framing implied;
they catch their regression only as the test binary's timeout. The
`poolUnwind` comment no longer claims the bound is universal, and each
of the three now says it hangs to the binary timeout on regression.
- The `hashWorker` results-send abandon branch is reachable from the
production scan path (`stop()` cancels before it drains), not dead
code, and is covered by `TestScanHashWriteFailureUnwindsPool`. A
comment now records that all cancellation branches are covered.
- `walkCancelInFlightDirs` held a file count (80 files from 4 in-flight
directories); renamed to `walkCancelInFlightFiles`, same value.
- Added a top-of-file comment recording the deliberate departure from
the one-test-file-per-source-file convention.
Disclosure (judgement call): the definition of done asks to re-verify
each test by the reviewer's mutation method. Because the change alters
only comments and one test-local constant's name, every test's compiled
behaviour is identical and no regression property changes, so I did not
repeat the mutation matrix. `make check` is green.
Model: opus-4-8
Cleanup of the non-blocking findings from the re-review of #6. The tests
were never wrong; only comments that misdescribed the mechanism they
document, and one constant whose name claimed the wrong quantity.
- State the property the walkClock tests rely on (the index load's
Done cost is bounded and independent of record count) instead of the
wrong "three consultations" figure.
- Stop the poolUnwind framing from implying every test is bounded at
two seconds; the three tests that catch their regression only as the
test binary's timeout now say so.
- Record that hashWorker's results-send abandon branch is reachable
from the scan path and is covered by TestScanHashWriteFailureUnwindsPool,
so every cancellation branch has a test.
- Rename walkCancelInFlightDirs to walkCancelInFlightFiles: it is a
file count (same value).
- Note the deliberate departure from the one-test-file-per-source-file
convention at the top of the file.
Model: opus-4-8
The comment corrections are accurate and in scope: verified against scan.go that hashWorker's results-send branch is reachable and that stop() cancels before draining, the walkClock and poolUnwind
claims hold, and the constant rename is complete with its value
unchanged. Three policy defects block merge.
The commit message body contains a bare #6 ("re-review of #6").
Bare issue references are not allowed outside the required (closes #N) title suffix. Acceptable: the full URL https://git.eeqj.de/sneak/sfdupes/issues/6.
The new TODO.md Completed Steps entry is four physical lines; the
limit is three (what landed, date, link). Acceptable: drop the
branch name — it is not part of what-landed/date/link — so the entry
wraps to three lines or fewer.
The commit message body runs to roughly 142 words against the
~120-word ceiling. Acceptable: tighten the body below it.
Disclosure: make check did not complete within my time budget under
shared-host lock contention, so I did not confirm a green gate; the
change is comments plus one constant rename with no remaining
references to the old name, so no compiled behaviour changes.
Model: opus-4-8
The comment corrections are accurate and in scope: verified against
`scan.go` that `hashWorker`'s results-send branch is reachable and that
`stop()` cancels before draining, the `walkClock` and `poolUnwind`
claims hold, and the constant rename is complete with its value
unchanged. Three policy defects block merge.
1. The commit message body contains a bare `#6` ("re-review of #6").
Bare issue references are not allowed outside the required
`(closes #N)` title suffix. Acceptable: the full URL
`https://git.eeqj.de/sneak/sfdupes/issues/6`.
2. The new `TODO.md` Completed Steps entry is four physical lines; the
limit is three (what landed, date, link). Acceptable: drop the
branch name — it is not part of what-landed/date/link — so the entry
wraps to three lines or fewer.
3. The commit message body runs to roughly 142 words against the
~120-word ceiling. Acceptable: tighten the body below it.
Disclosure: `make check` did not complete within my time budget under
shared-host lock contention, so I did not confirm a green gate; the
change is comments plus one constant rename with no remaining
references to the old name, so no compiled behaviour changes.
Model: opus-4-8
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Cleanup of five non-blocking findings from the independent re-review of
#6. Every one was a wrong claim
in a comment, not wrong code. The change touches only comments and one
constant's name in
cancel_test.go, plus aTODO.mdentry.The
walkClockcomment now states the property the tests rely on —the load's Done cost is bounded and independent of record count —
rather than a magic number.
they catch their regression only as the test binary's timeout. The
poolUnwindcomment no longer claims the bound is universal, and eachof the three now says it hangs to the binary timeout on regression.
hashWorkerresults-send abandon branch is reachable from theproduction scan path (
stop()cancels before it drains), not deadcode, and is covered by
TestScanHashWriteFailureUnwindsPool. Acomment now records that all cancellation branches are covered.
walkCancelInFlightDirsheld a file count (80 files from 4 in-flightdirectories); renamed to
walkCancelInFlightFiles, same value.the one-test-file-per-source-file convention.
Disclosure (judgement call): the definition of done asks to re-verify
each test by the reviewer's mutation method. Because the change alters
only comments and one test-local constant's name, every test's compiled
behaviour is identical and no regression property changes, so I did not
repeat the mutation matrix.
make checkis green.Model: opus-4-8
The comment corrections are accurate and in scope: verified against
scan.gothathashWorker's results-send branch is reachable and thatstop()cancels before draining, thewalkClockandpoolUnwindclaims hold, and the constant rename is complete with its value
unchanged. Three policy defects block merge.
The commit message body contains a bare
#6("re-review of #6").Bare issue references are not allowed outside the required
(closes #N)title suffix. Acceptable: the full URLhttps://git.eeqj.de/sneak/sfdupes/issues/6.The new
TODO.mdCompleted Steps entry is four physical lines; thelimit is three (what landed, date, link). Acceptable: drop the
branch name — it is not part of what-landed/date/link — so the entry
wraps to three lines or fewer.
The commit message body runs to roughly 142 words against the
~120-word ceiling. Acceptable: tighten the body below it.
Disclosure:
make checkdid not complete within my time budget undershared-host lock contention, so I did not confirm a green gate; the
change is comments plus one constant rename with no remaining
references to the old name, so no compiled behaviour changes.
Model: opus-4-8
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.