Skip to content

fix(pre-commit): replace pre-commit with shell git hooks and update documentation #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wambitz merged 5 commits into from
Mar 12, 2026
Merged

fix(pre-commit): replace pre-commit with shell git hooks and update documentation #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3930443
fix: replace pre-commit with plain shell git hooks
wambitz Mar 11, 2026
564462f
docs: update README and devcontainer for new git hooks setup
wambitz Mar 11, 2026
596b455
fix: skip clang-tidy in pre-push hook when build dir is missing
wambitz Mar 12, 2026
e25dfb4
refactor: single Docker invocation for pre-push hook
wambitz Mar 12, 2026
9988003
fix: drain stdin in pre-push hook and fix install-hooks comment
wambitz Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
}
},

"postCreateCommand": "pre-commit install --hook-type pre-push",
"postCreateCommand": "./scripts/install-hooks.sh",
"postStartCommand": "bash",

// Clean container name and setup
Expand Down
29 changes: 29 additions & 0 deletions .githooks/pre-push
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env bash
set -e

###############################################################################
# Git pre-push hook
#
# Runs formatting and linting checks before pushing. Delegates to Docker
# as a single container invocation (via the same mechanism as other scripts).
###############################################################################

# Drain stdin — git pipes ref info here; leaving it unconsumed can block.
cat > /dev/null

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../scripts" && pwd)"
source "$SCRIPT_DIR/env.sh"
source "$SCRIPT_DIR/docker/exec.sh"
delegate_to_container "$@"

log_info "Pre-push: checking formatting..."
"$PROJECT_ROOT/scripts/format.sh" --check
Comment on lines +16 to +20
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

BUILD_DIR="$PROJECT_ROOT/build"
if [ ! -f "$BUILD_DIR/compile_commands.json" ]; then
log_warn "Pre-push: skipping clang-tidy (no compile_commands.json)"
log_warn "Run ./scripts/build.sh first to enable lint checks."
else
log_info "Pre-push: running clang-tidy..."
"$PROJECT_ROOT/scripts/lint.sh"
fi
19 changes: 0 additions & 19 deletions .pre-commit-config.yaml
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
./scripts/docs.sh # Generate Doxygen documentation
```

Pre-commit hooks run on **pre-push** (not pre-commit): `pre-commit install --hook-type pre-push`
Git hooks live in `.githooks/` (version-controlled). Install once after cloning: `./scripts/install-hooks.sh` (runs directly on the host).

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.
Build and quality 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.

## Architecture

Expand Down
5 changes: 0 additions & 5 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ RUN apt-get update && apt-get install -y \
lcov \
gdb \
valgrind \
python3-pip \
ca-certificates \
tree \
&& rm -rf /var/lib/apt/lists/*
Expand Down Expand Up @@ -70,10 +69,6 @@ RUN echo 'ubuntu ALL=(ALL) NOPASSWD:ALL' > /etc/sudoers.d/ubuntu \
# ------------------------------------------------------------------------------
# RUN sed -i 's/^#force_color_prompt=yes/force_color_prompt=yes/' /home/ubuntu/.bashrc

# -----------------------------------------------------------------------------
# Install pre-commit for code quality checks
# -----------------------------------------------------------------------------
RUN pip3 install --break-system-packages pre-commit cmake-format

# ------------------------------------------------------------------------------
# Copy entrypoint script
Expand Down
Loading
Loading