Skip to content

Add pre-commit hooks, excluding the binary test fixtures - #76

Merged
psobot merged 1 commit into
masterfrom
psobot/pre-commit
Aug 8, 2026
Merged

psobot merged 1 commit into
masterfrom
psobot/pre-commit

Conversation

@psobot

@psobot psobot commented Aug 7, 2026 •

Copy link
Copy Markdown
Owner

No description provided.

The hooks are worth having, but they cannot be pointed at tests/data/. The
.iwa and .key fixtures are binary, yet `identify` finds no NUL bytes near the
start of some of them and classifies them as text, so trailing-whitespace
strips any 0x09 that happens to precede a 0x0a - which inside a Snappy stream
or an embedded JPEG quantization table is just data, not whitespace. The
archive is silently corrupted and the only symptom is a zipfile.BadZipFile in
an unrelated test run much later.

Verified by running the hooks over tests/data/ without the exclude: three .key
fixtures, one .yaml fixture and tests/data/table/Metadata/DocumentIdentifier
were all modified. Excluding the directory rather than a *.key glob is
deliberate - the .iwa files have the same problem, and the .yaml fixtures are
compared byte-for-byte by tests/test_codec.py.

ruff is pinned to the same 0.16.2 as .github/workflows/python-package.yml, and
picks up the rule selection from pyproject.toml, so the hook and CI agree about
what passes. Letting those drift is how the lint gate stopped meaning anything
the last time round.

pyright is deliberately absent: it reports 43 errors on master today, so
including it would block every commit rather than catch anything new. Worth
adding once those are dealt with.

The first run also normalized missing trailing newlines in eight .proto files
and docs/obriensp_docs.md. Confirmed inert: recompiling the protos before and
after produces byte-identical generated code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179M4xvAKPGrgKpy4AeCsM7
@psobot
psobot merged commit 24a6d76 into master Aug 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant