Skip to content

chore(pre-commit): arm the pre-push hooks on a plain install - #2223

Merged
cristim merged 1 commit into
mainfrom
chore/pre-commit-install-both-stages
Oct 16, 2026
Merged

cristim merged 1 commit into
mainfrom
chore/pre-commit-install-both-stages

Conversation

@cristim

@cristim cristim commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Closes #1113

Summary

.pre-commit-config.yaml sets default_stages: [pre-commit, pre-push] but did not set default_install_hook_types. So a plain pre-commit install wired only the pre-commit git hook, and the pre-push stage, including go test -short -race ./..., silently never ran locally.

  • Added default_install_hook_types: [pre-commit, pre-push].
  • Updated the go-test hook comment, which told developers to run pre-commit install --hook-type pre-push.

For contributors: re-run pre-commit install once to arm the pre-push hook. After that, git push also runs the pre-push stage locally, including the race-enabled short test suite.

CI is unaffected. It never runs pre-commit install, and its pre-commit run --all-files uses the default pre-commit stage (.github/workflows/pre-commit.yml:177).

Evidence (local, macOS, pre-commit 4.7.0; CI pins 4.0.1, and the key exists since 2.18)

  • pre-commit validate-config passes.
  • In a throwaway repo, a plain pre-commit install with this config creates both .git/hooks/pre-commit and .git/hooks/pre-push. With the origin/main config it creates only pre-commit.
  • Pre-commit hooks pass on the commit.
  • An independent non-author reviewer (sonnet, high) did two passes. Pass 1 found no blocking issues. Pass 2 raised one optional low (the stale hook comment), which is fixed. The final check found no actionable findings at 11ea46b.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Installing the repository’s commit hooks now also enables pre-push hooks by default. Go tests continue to run before pushing changes.

default_stages includes pre-push, but without default_install_hook_types
a plain `pre-commit install` wired only the pre-commit git hook, so the
pre-push stage (including go test -short -race) silently never ran
locally. Set default_install_hook_types and update the hook comment that
told developers to pass --hook-type pre-push.

Closes #1113

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline impact/internal Team-internal only effort/xs Trivial / one-liner type/chore Maintenance / non-user-visible triaged Item has been triaged labels Oct 16, 2026
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: ca2c1ffa-2a9c-407e-8484-d79576e006e8
📥 Commits

Reviewing files that changed from the base of the PR and between e8543f0 and 11ea46b.

📒 Files selected for processing: 1
  • .pre-commit-config.yaml

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

The pre-commit configuration now sets both pre-commit and pre-push as default installation hook types. Comments describe the local pre-push hook and the scope of CI checks.

Changes: Hook installation configuration

Layer / File(s) Summary
Default hook installation
.pre-commit-config.yaml
The configuration sets pre-commit and pre-push as default hook types for installation. Comments state that a plain pre-commit install arms the pre-push hook and describe the CI checks.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 11ea4

A plain local pre-commit install arms both hooks, including the Go test on pre-push; CI retains its existing default pre-commit checks. The inspected configuration is consistent with that behavior, so no actionable merge risk remains.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: configuring a plain pre-commit install to enable pre-push hooks.
Linked Issues check Passed The diff adds default_install_hook_types: [pre-commit, pre-push] to .pre-commit-config.yaml, as required by issue #1113. The change is placed with default_stages. The PR evidence reports that `p…
Out of Scope Changes check Passed The PR changes only .pre-commit-config.yaml. The comment update explains the new hook installation behavior and supports issue #1113. No unrelated repository or implementation changes are present in…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Full details: Linked Issues check

Explanation

The diff adds default_install_hook_types: [pre-commit, pre-push] to .pre-commit-config.yaml, as required by issue #1113. The change is placed with default_stages. The PR evidence reports that pre-commit validate-config passes.

Full details: Out of Scope Changes check

Explanation

The PR changes only .pre-commit-config.yaml. The comment update explains the new hook installation behavior and supports issue #1113. No unrelated repository or implementation changes are present in the reviewed diff.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Independent review verdict: CLEAN at 11ea46b

Reviewer: fresh Claude agent (Sonnet, high effort), not the author. Coordinator: claude-cc-reviewer.

What was checked:

  • The diff only adds default_install_hook_types: [pre-commit, pre-push] and edits a comment.
  • The only stages: [pre-push] hook (go-test, line 204) is unchanged, and no other stage changed.
  • pre-commit validate-config passes.
  • In a scratch repo, a plain pre-commit install created both .git/hooks/pre-commit and .git/hooks/pre-push.
  • .github/workflows/README.md:80 already documents plain pre-commit install, so it is now accurate.

Gate:

@cristim
cristim merged commit e02448a into main Oct 16, 2026
12 checks passed
@cristim
cristim deleted the chore/pre-commit-install-both-stages branch October 16, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(pre-commit): set default_install_hook_types so install arms both stages

1 participant