Skip to content

[build] Add pre-push formatter hook#17455

Open
AutomatedTester wants to merge 1 commit into
trunkfrom
elated-sinoussi-2d381c
Open

[build] Add pre-push formatter hook#17455
AutomatedTester wants to merge 1 commit into
trunkfrom
elated-sinoussi-2d381c

Conversation

@AutomatedTester
Copy link
Copy Markdown
Member

Summary

  • Adds .githooks/pre-push — a repository-managed hook that runs ./scripts/format.sh --pre-push before every push.
  • If formatters modify files the hook auto-commits them (chore: apply formatters before push) and aborts the push, asking the contributor to re-push with the clean state included.
  • If the working tree is dirty the hook aborts immediately to avoid mixing uncommitted work with the formatter output.
  • Documents the one-time setup step (git config core.hooksPath .githooks) and expected push behavior in CONTRIBUTING.md under Step 6: Push.

Motivation

CI runs ./go format and fails if git diff is non-empty afterwards. Contributors frequently hit this only after opening a PR. This hook catches the issue locally, creates the fix commit automatically, and surfaces a clear re-push message — removing the round-trip.

Reviewer notes

  • The hook is opt-in: contributors must run git config core.hooksPath .githooks once per clone. Nothing changes for anyone who does not set this.
  • SKIP_FORMAT_HOOK=1 guards the auto-commit path to prevent hook recursion.
  • --no-verify remains available as an escape hatch (documented as discouraged since CI will still fail).
  • The hook delegates all formatter logic to the existing scripts/format.sh; no formatter code is duplicated.

Test plan

  • Clone the repo, set core.hooksPath, make a change that requires formatting, and verify the hook commits the fix and asks to re-push.
  • Verify second push succeeds with no additional formatter commits.
  • Verify a dirty working tree is rejected before any formatters run.
  • Verify git push --no-verify skips the hook entirely.

Adds .githooks/pre-push so contributors can auto-fix formatting issues
before they reach CI. The hook calls scripts/format.sh --pre-push; if
formatters modify files it stages them, commits them, and asks the user
to re-push — keeping the branch clean without requiring a manual format
step. Documents one-time setup (git config core.hooksPath .githooks) in
CONTRIBUTING.md under Step 6: Push.
@diemol
Copy link
Copy Markdown
Member

diemol commented May 14, 2026

@titusfortner
Copy link
Copy Markdown
Member

I think I'm ok with a pre-push hook in theory, but...

  1. we would need to support both sh and ps1
  2. I don't like the auto-commit. The reason we have a auto-commit step in the CI is because it would take forever to get the failure have the submitter see the problem, fix the problem, rerun it, when it is a simple formatting thing. If we can alert the user real time to an issue, they can commit it themselves and submit, not a heavy lift.
  3. I'm not sure we should assume that everyone looking to contribute has a working bazel setup

My proposal is that we just encourage people to create their own pre-push or pre-commit hooks.

My pre-push hook is just:

#!/usr/bin/env bash
./scripts/format.sh --pre-push --lint

A separate note, I kind of think --pre-push should default to --lint behavior

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.

3 participants