Skip to content

chore(make): auto-install pre-commit hooks on lint #3187

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
ralphbean merged 2 commits into from
Jul 8, 2026
Merged

chore(make): auto-install pre-commit hooks on lint #3187

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3b8418
chore(make): auto-install pre-commit hooks on lint
ralphbean Jul 6, 2026
0b88574
fix(make): use git-aware hooks path in ensure-hooks
ralphbean Jul 7, 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
12 changes: 9 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.DEFAULT_GOAL := help
.PHONY: help bootstrap lint lint-all check fmt \
.PHONY: help bootstrap ensure-hooks lint lint-all check fmt \
mindmap go-build go-test go-lint go-fmt go-vet go-tidy \
lint-md-links script-test test \
e2e-test behaviour-test lint-eval-cases functional-tests
Expand Down Expand Up @@ -71,10 +71,16 @@ bootstrap:
@echo "==> Bootstrap complete!"
@echo " Make sure $(BOOTSTRAP_BIN_DIR) is on your PATH."

lint:
ensure-hooks:

@fullsend-ai-review fullsend-ai-review Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] documentation-missing

The new ensure-hooks target lacks documentation in the help target. All other .PHONY targets in this Makefile have corresponding help entries (lines 13-33), but ensure-hooks is missing. While it is an internal dependency target not meant to be invoked directly, the established pattern is to document all .PHONY targets.

Suggested fix: Add a help entry, e.g.: @echo " ensure-hooks - Install pre-commit hooks if not present"

@fullsend-ai-review fullsend-ai-review Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] scope-creep

The change makes make lint auto-install pre-commit hooks on first run, converting it from a pure check into an idempotent install+check operation. AGENTS.md guidance to always stage changes before running make lint could benefit from a note about the new auto-install behavior, though the staging advice itself remains valid regardless.

Suggested fix: Consider updating AGENTS.md to mention that make lint now also installs hooks if missing.

@if [ ! -f .git/hooks/pre-commit ]; then \
echo "==> Installing pre-commit hooks..."; \
pre-commit install; \
fi
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

lint: ensure-hooks
pre-commit run

lint-all:
lint-all: ensure-hooks
pre-commit run --all-files

check:
Expand Down
Loading