From 3db6cd430fcf7716fe9e7d7b628de89b7bce8701 Mon Sep 17 00:00:00 2001 From: Ashish Date: Tue, 9 Jun 2026 21:09:22 -0700 Subject: [PATCH] chore: wire pre-commit ruff hooks into make install-git-hooks (#786) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `.pre-commit-config.yaml` already has `ruff` + `ruff-format` configured, and `pre-commit>=3.0.0` is already in `[dev]` deps — but `make install-git-hooks` never called `pre-commit install`. Every contributor's repo had the hook **config** but no running hook. PR #772 merged with inline-comment spacing and import-order violations that ruff would have caught automatically. The maintainer had to add a separate fixup commit (`fix: format issue 728 regression test`) to clean it up. ## Changes **`scripts/install-git-hooks.sh`** — after installing the pre-push hook, also run `pre-commit install`. Falls back to `.venv/bin/pre-commit` when `pre-commit` is not on `PATH`, with a clear warning if neither is found: ``` ✅ installed: .git/hooks/pre-push Runs 'make ci-precheck' before every git push. ✅ installed: .git/hooks/pre-commit (ruff lint + format via pre-commit) ``` **`CONTRIBUTING.md`** — update PR workflow step 2 to mention `make install-git-hooks` so contributors know to run it after `pip install`: ``` 2. pip install -e ".[dev]" then make install-git-hooks — installs ruff on every commit and ci-precheck on every push. ``` ## No behaviour change for existing code Only the local dev setup script is touched. Nothing in the proxy, tests, or CI pipeline changes. ## Real behavior proof - **OS**: macOS darwin arm64 - **Steps**: ran `bash scripts/install-git-hooks.sh` with venv available, then attempted a commit with a badly-formatted file - **Result**: ruff caught and auto-fixed it before the commit landed ``` ✅ installed: .git/hooks/pre-push Runs 'make ci-precheck' before every git push. Bypass (use sparingly): git push --no-verify pre-commit installed at .git/hooks/pre-commit ✅ installed: .git/hooks/pre-commit (ruff lint + format via pre-commit) ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 4.6 --- CONTRIBUTING.md | 2 +- scripts/install-git-hooks.sh | 42 ++++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6003ac3f0..3ccf8bb10 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,7 +73,7 @@ A human maintainer reviews every dep change. PRs that add or bump a package must ## PR workflow 1. Fork, branch from `main`. -2. `pip install -e ".[dev]"` +2. `pip install -e ".[dev]"` then `make install-git-hooks` — installs repo pre-commit checks on every commit and ci-precheck on every push. 3. One logical change per PR. 4. Add tests. 5. `pytest` · `ruff check .` · `ruff format .` diff --git a/scripts/install-git-hooks.sh b/scripts/install-git-hooks.sh index 699fed1cf..53975da22 100755 --- a/scripts/install-git-hooks.sh +++ b/scripts/install-git-hooks.sh @@ -1,15 +1,19 @@ #!/usr/bin/env bash -# Install a pre-push git hook that runs `make ci-precheck` before every push. +# Install git hooks for the Headroom repo: +# 1. pre-commit — repo pre-commit checks (ruff, mypy, sync-plugin-versions) +# 2. pre-push — full ci-precheck (cargo fmt/clippy/test + python suite) # -# Why: the 2026-04-27 push hit five CI failures that could all have been -# caught locally — cargo fmt drift, an x86_64-apple-darwin wheel that the -# project doesn't actually need, missing Rust extension in two CI lanes, -# and a commitlint warning treated as an error. The fixes are committed; -# this hook ensures we don't repeat the same dance. +# Why pre-push was added: the 2026-04-27 push hit five CI failures that could +# all have been caught locally — cargo fmt drift, an x86_64-apple-darwin wheel +# that the project doesn't actually need, missing Rust extension in two CI +# lanes, and a commitlint warning treated as an error. # -# Idempotent. Re-running is safe — it overwrites the hook file with the -# current desired contents. Skips installation if `.git/hooks/` is missing -# (e.g. running outside a git checkout). +# Why pre-commit was added: PR #772 merged with inline-comment spacing and +# import-order violations because the ruff pre-commit hook in +# .pre-commit-config.yaml was never installed for contributors. +# +# Idempotent. Re-running is safe. Skips installation if `.git/hooks/` is +# missing (e.g. running outside a git checkout). set -euo pipefail @@ -70,3 +74,23 @@ chmod +x "$HOOK_PATH" echo "✅ installed: $HOOK_PATH" echo " Runs 'make ci-precheck' before every git push." echo " Bypass (use sparingly): git push --no-verify" + +# Install pre-commit hooks (repo checks on every commit). +# Prefer the project venv over a global install so contributors always run the +# pinned version. Resolution order: active $VIRTUAL_ENV → .venv → global PATH. +PRE_COMMIT_BIN="" +if [[ -n "${VIRTUAL_ENV:-}" && -x "${VIRTUAL_ENV}/bin/pre-commit" ]]; then + PRE_COMMIT_BIN="${VIRTUAL_ENV}/bin/pre-commit" +elif [[ -x .venv/bin/pre-commit ]]; then + PRE_COMMIT_BIN=".venv/bin/pre-commit" +elif command -v pre-commit &>/dev/null; then + PRE_COMMIT_BIN="pre-commit" +fi + +if [[ -n "$PRE_COMMIT_BIN" ]]; then + "$PRE_COMMIT_BIN" install + echo "✅ installed: .git/hooks/pre-commit (repo pre-commit checks via pre-commit)" +else + echo "error: pre-commit not found — run 'pip install -e .[dev]' first, then re-run this script." >&2 + exit 1 +fi