fix(pre-commit): replace pre-commit with shell git hooks and update documentation by wambitz · Pull Request #43 · wambitz/cpp-template

wambitz · 2026-03-11T23:55:11Z

Summary

This pull request refactors the project's git hook setup, replacing the previous pre-commit-based approach with custom, version-controlled git hooks for formatting and linting checks. The new hooks are easier to install, are tracked in the repository, and delegate to Docker when run outside the container. Documentation and configuration files are updated to reflect these changes.

Git hook system overhaul:

Removed pre-commit configuration for formatting and linting checks from .pre-commit-config.yaml, discontinuing use of pre-commit for pre-push hooks.
Added a version-controlled pre-push hook in .githooks/pre-push that runs formatting and linting scripts, auto-delegating to Docker if needed.
Introduced scripts/install-hooks.sh to set core.hooksPath to .githooks, ensuring git uses the project's hooks; updated DevContainer to use this script for hook installation. [1] [2]

Documentation updates:

Updated README.md and CLAUDE.md to describe the new git hook installation and workflow, replacing references to pre-commit with instructions for the new system. [1] [2] [3] [4]

Container and dependency cleanup:

Removed installation of python3-pip and pre-commit from Dockerfile, as they are no longer required for code quality checks. [1] [2]

Resolves #42

Remove Python-based pre-commit dependency in favor of version-controlled .githooks/ directory with core.hooksPath. This eliminates the need for Python/pip on the host while keeping the same pre-push checks (format and lint) via Docker delegation. - Add .githooks/pre-push hook calling format.sh and lint.sh directly - Add scripts/install-hooks.sh for one-command setup after cloning - Remove .pre-commit-config.yaml - Remove pre-commit, cmake-format, and python3-pip from Dockerfile - Update CLAUDE.md with new hook install instructions

Replace pre-commit references with install-hooks.sh in the Quick Start, Code Quality section, and devcontainer postCreateCommand.

Copilot

Pull request overview

This PR replaces the repo’s pre-commit-based pre-push checks with a version-controlled Git hooks setup that runs the existing formatting/linting scripts (which already auto-delegate to Docker), and updates container setup + docs accordingly.

Changes:

Removed the pre-commit configuration and related container dependencies.
Added a version-controlled .githooks/pre-push hook and a scripts/install-hooks.sh installer that sets core.hooksPath.
Updated README/CLAUDE/DevContainer configuration to document and auto-install the new hook setup.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`scripts/install-hooks.sh`	Adds a one-shot installer that configures Git to use `.githooks/`.
`README.md`	Updates quickstart and code-quality docs to reference the new pre-push hook workflow.
`Dockerfile`	Removes pip / pre-commit installation since checks now run via shell scripts/hooks.
`CLAUDE.md`	Updates contributor guidance to point to the new hook installation command.
`.pre-commit-config.yaml`	Removes the old pre-commit hook configuration.
`.githooks/pre-push`	Adds a pre-push hook that runs `format.sh --check` and `lint.sh`.
`.devcontainer/devcontainer.json`	Installs hooks automatically in the DevContainer via `postCreateCommand`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T23:58:06Z

.githooks/pre-push

+REPO_ROOT="$(git rev-parse --show-toplevel)"
+
+echo "=== Pre-push: checking formatting ==="
+"$REPO_ROOT/scripts/format.sh" --check
+


The hook runs scripts/lint.sh unconditionally. lint.sh relies on an existing build directory with compile_commands.json (via -p "$BUILD_DIR"), so this hook will fail on fresh clones / before a first CMake configure. Consider adding a preflight check for build/compile_commands.json and emitting a clear instruction (e.g., run cmake -S . -B build) before exiting non-zero (or explicitly skipping lint).

Suggested change

REPO_ROOT="$(git rev-parse --show-toplevel)"

echo "=== Pre-push: checking formatting ==="

"$REPO_ROOT/scripts/format.sh" --check

REPO_ROOT="$(git rev-parse --show-toplevel)"

BUILD_DIR="$REPO_ROOT/build"

echo "=== Pre-push: checking formatting ==="

"$REPO_ROOT/scripts/format.sh" --check

# Ensure clang-tidy prerequisites exist before running lint.

if [ ! -f "$BUILD_DIR/compile_commands.json" ]; then

echo "=== Pre-push: skipping clang-tidy ==="

echo "No compile_commands.json found at '$BUILD_DIR/compile_commands.json'."

echo "Run: cmake -S \"$REPO_ROOT\" -B \"$BUILD_DIR\""

exit 1

fi

Copilot · 2026-03-11T23:58:06Z

README.md

+This sets `core.hooksPath` to `.githooks/`, so git uses the project's hooks. Before each push, the hook will:
 - Run `clang-format` to check code formatting
 - Run `clang-tidy` to analyze code quality

-These checks ensure consistent code style across the team.
-
-**VS Code DevContainer users:** Hooks are installed automatically via `postCreateCommand`.
+These checks delegate to Docker automatically (same as running the scripts directly).


This section no longer mentions that clang-tidy requires a configured build directory with compile_commands.json (see scripts/lint.sh, which runs clang-tidy ... -p build). Since the pre-push hook now runs on every push, it’s easy for contributors to hit confusing failures unless the docs call out the “build first” requirement (or the hook performs a preflight check).

Add preflight check for compile_commands.json before running lint. Format check still runs unconditionally. Update README to document the build requirement for lint.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T00:25:39Z

README.md

+## Scripts

-### CMake
+All scripts auto-delegate to Docker when run on the host. Inside the container or CI (`CI=true`), they run directly.


This section states all scripts auto-delegate to Docker, but scripts/install-hooks.sh is a counterexample (it runs directly on the host and does not delegate). Please adjust the wording here to match actual behavior, or update install-hooks.sh to follow the same delegation contract as the other scripts.

Suggested change

All scripts auto-delegate to Docker when run on the host. Inside the container or CI (`CI=true`), they run directly.

All build, test, coverage, formatting, linting, and docs scripts auto-delegate to Docker when run on the host. Inside the container or CI (`CI=true`), they run directly. The `install-hooks.sh` script always runs on the host to configure Git hooks.

Copilot · 2026-03-12T00:25:40Z

scripts/install-hooks.sh

+REPO_ROOT="$(dirname "$SCRIPT_DIR")"
+
+git -C "$REPO_ROOT" config core.hooksPath .githooks
+echo "[INFO] Git hooks installed (.githooks/). core.hooksPath set."


install-hooks.sh is the only script under scripts/ that doesn’t follow the repo’s established pattern of sourcing scripts/env.sh for consistent PROJECT_ROOT resolution + log helpers (and most scripts also source scripts/docker/exec.sh). Consider sourcing env.sh and using PROJECT_ROOT/log_info to avoid duplicating root detection and to keep script output consistent with the rest of the tooling.

Suggested change

REPO_ROOT="$(dirname "$SCRIPT_DIR")"

git -C "$REPO_ROOT" config core.hooksPath .githooks

echo "[INFO] Git hooks installed (.githooks/). core.hooksPath set."

# Source shared environment helpers for PROJECT_ROOT and logging

# shellcheck source=env.sh

. "${SCRIPT_DIR}/env.sh"

git -C "$PROJECT_ROOT" config core.hooksPath .githooks

log_info "Git hooks installed (.githooks/). core.hooksPath set."

Copilot · 2026-03-12T00:25:40Z

.githooks/pre-push

+REPO_ROOT="$(git rev-parse --show-toplevel)"
+
+echo "=== Pre-push: checking formatting ==="
+"$REPO_ROOT/scripts/format.sh" --check
+
+BUILD_DIR="$REPO_ROOT/build"
+if [ ! -f "$BUILD_DIR/compile_commands.json" ]; then
+    echo "=== Pre-push: skipping clang-tidy (no compile_commands.json) ==="
+    echo "Run ./scripts/build.sh first to enable lint checks."
+else
+    echo "=== Pre-push: running clang-tidy ==="
+    "$REPO_ROOT/scripts/lint.sh"


On hosts, this hook will typically start Docker twice (once via format.sh delegation and again via lint.sh delegation). Since scripts/docker/exec.sh uses docker run --rm each time, this adds noticeable overhead to every git push. Consider delegating once at the hook level (source scripts/env.sh + scripts/docker/exec.sh and call delegate_to_container at the top) and then run both checks inside that single container invocation.

Copilot · 2026-03-12T00:25:40Z

CLAUDE.md

+Git hooks live in `.githooks/` (version-controlled). Install once after cloning: `./scripts/install-hooks.sh`

 All scripts auto-delegate to Docker when run outside the container (`docker run --rm`). The delegation logic lives in `scripts/docker/exec.sh`, sourced by each script via `scripts/env.sh`. Inside the container or CI (`CI=true`), scripts run directly with no overhead. See `docs/ci-container-delegation.md` for details on the CI strategy and a GHCR upgrade path.


This section mentions that all scripts auto-delegate to Docker when run outside the container, but scripts/install-hooks.sh currently runs directly on the host (no delegate_to_container). Please either adjust the wording to exclude hook installation, or change install-hooks.sh to conform so the guidance here matches actual behavior.

Suggested change

Git hooks live in `.githooks/` (version-controlled). Install once after cloning: `./scripts/install-hooks.sh`

All scripts auto-delegate to Docker when run outside the container (`docker run --rm`). The delegation logic lives in `scripts/docker/exec.sh`, sourced by each script via `scripts/env.sh`. Inside the container or CI (`CI=true`), scripts run directly with no overhead. See `docs/ci-container-delegation.md` for details on the CI strategy and a GHCR upgrade path.

Git hooks live in `.githooks/` (version-controlled). Install once after cloning: `./scripts/install-hooks.sh` (runs directly on the host).

All build/test/utility scripts above auto-delegate to Docker when run outside the container (`docker run --rm`). The delegation logic lives in `scripts/docker/exec.sh`, sourced by each script via `scripts/env.sh`. Inside the container or CI (`CI=true`), scripts run directly with no overhead. See `docs/ci-container-delegation.md` for details on the CI strategy and a GHCR upgrade path.

Copilot · 2026-03-12T00:25:40Z

README.md

+./scripts/test.sh
 ```

+Every script in `scripts/` auto-delegates to Docker when run from the host. You don't need CMake, clang-format, or any other tool installed locally — just Docker. See [docs/ci-container-delegation.md](docs/ci-container-delegation.md) for details.


The README claims "Every script in scripts/ auto-delegates to Docker", but scripts/install-hooks.sh does not source scripts/docker/exec.sh or call delegate_to_container, so this statement is currently inaccurate. Either update the wording to exclude hook-installation (e.g., "build/quality scripts"), or make install-hooks.sh follow the same delegation pattern if that’s the intended contract.

Suggested change

Every script in `scripts/` auto-delegates to Docker when run from the host. You don't need CMake, clang-format, or any other tool installed locally — just Docker. See [docs/ci-container-delegation.md](docs/ci-container-delegation.md) for details.

Build and quality scripts in `scripts/` auto-delegate to Docker when run from the host. You don't need CMake, clang-format, or any other tool installed locally — just Docker. See [docs/ci-container-delegation.md](docs/ci-container-delegation.md) for details.

- Pre-push hook now sources env.sh + docker/exec.sh and delegates itself, so both format and lint run in one container (not two) - install-hooks.sh sources env.sh for consistent logging - Fix "all scripts" wording in README and CLAUDE.md to clarify that install-hooks.sh runs directly on the host

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T00:48:26Z

.githooks/pre-push

+source "$SCRIPT_DIR/docker/exec.sh"
+delegate_to_container "$@"
+
+log_info "Pre-push: checking formatting..."
+"$PROJECT_ROOT/scripts/format.sh" --check


git pre-push provides a list of refs to push on stdin. This hook never consumes stdin, and when running on the host it delegates to docker run (which is started without -i), so git can block if the stdin pipe fills on pushes with many refs/tags. Drain stdin (even if you ignore it) before invoking delegate_to_container, or adjust delegation to forward stdin into the container so the pipe is consumed.

Copilot · 2026-03-12T00:48:27Z

scripts/install-hooks.sh

+#
+#   ./scripts/install-hooks.sh
+#
+# This script runs directly on the host (no Docker delegation).


The header comment says this script runs "directly on the host", but it’s now invoked from the DevContainer postCreateCommand (which runs inside the container). Consider rewording to "runs directly (no Docker delegation)" to avoid implying it only works on the host.

Suggested change

# This script runs directly on the host (no Docker delegation).

# This script runs directly (no Docker delegation).

- Drain git's ref info from stdin before delegation to prevent pipe blocking on pushes with many refs - Reword install-hooks.sh comment since it also runs inside containers via devcontainer postCreateCommand

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

wambitz added 2 commits March 11, 2026 15:58

docs: update README and devcontainer for new git hooks setup

564462f

Replace pre-commit references with install-hooks.sh in the Quick Start, Code Quality section, and devcontainer postCreateCommand.

wambitz self-assigned this Mar 11, 2026

wambitz added the bug Something isn't working label Mar 11, 2026

Copilot AI review requested due to automatic review settings March 11, 2026 23:55

Copilot started reviewing on behalf of wambitz March 11, 2026 23:55 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

fix: skip clang-tidy in pre-push hook when build dir is missing

596b455

Add preflight check for compile_commands.json before running lint. Format check still runs unconditionally. Update README to document the build requirement for lint.

wambitz requested a review from Copilot March 12, 2026 00:21

Copilot started reviewing on behalf of wambitz March 12, 2026 00:21 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

wambitz requested a review from Copilot March 12, 2026 00:45

Copilot started reviewing on behalf of wambitz March 12, 2026 00:46 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

fix: drain stdin in pre-push hook and fix install-hooks comment

9988003

- Drain git's ref info from stdin before delegation to prevent pipe blocking on pushes with many refs - Reword install-hooks.sh comment since it also runs inside containers via devcontainer postCreateCommand

wambitz requested a review from Copilot March 12, 2026 00:56

Copilot started reviewing on behalf of wambitz March 12, 2026 00:57 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

wambitz merged commit b6568b6 into main Mar 12, 2026
9 checks passed

wambitz deleted the fix/42/replace-pre-commit-with-githooks branch March 12, 2026 01:00

	All scripts auto-delegate to Docker when run on the host. Inside the container or CI (`CI=true`), they run directly.
	All build, test, coverage, formatting, linting, and docs scripts auto-delegate to Docker when run on the host. Inside the container or CI (`CI=true`), they run directly. The `install-hooks.sh` script always runs on the host to configure Git hooks.

-REPO_ROOT="$(dirname "$SCRIPT_DIR")"
-git -C "$REPO_ROOT" config core.hooksPath .githooks
-echo "[INFO] Git hooks installed (.githooks/). core.hooksPath set."
+# Source shared environment helpers for PROJECT_ROOT and logging
+# shellcheck source=env.sh
+. "${SCRIPT_DIR}/env.sh"
+git -C "$PROJECT_ROOT" config core.hooksPath .githooks
+log_info "Git hooks installed (.githooks/). core.hooksPath set."

		Git hooks live in `.githooks/` (version-controlled). Install once after cloning: `./scripts/install-hooks.sh`

		All scripts auto-delegate to Docker when run outside the container (`docker run --rm`). The delegation logic lives in `scripts/docker/exec.sh`, sourced by each script via `scripts/env.sh`. Inside the container or CI (`CI=true`), scripts run directly with no overhead. See `docs/ci-container-delegation.md` for details on the CI strategy and a GHCR upgrade path.

	Every script in `scripts/` auto-delegates to Docker when run from the host. You don't need CMake, clang-format, or any other tool installed locally — just Docker. See [docs/ci-container-delegation.md](docs/ci-container-delegation.md) for details.
	Build and quality scripts in `scripts/` auto-delegate to Docker when run from the host. You don't need CMake, clang-format, or any other tool installed locally — just Docker. See [docs/ci-container-delegation.md](docs/ci-container-delegation.md) for details.

	# This script runs directly on the host (no Docker delegation).
	# This script runs directly (no Docker delegation).

Conversation

wambitz commented Mar 11, 2026

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants