Skip to content

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

Merged
cristim merged 1 commit into
mainfrom
chore/10-install-hook-types
Oct 16, 2026
Merged

cristim merged 1 commit into
mainfrom
chore/10-install-hook-types

Conversation

@cristim

@cristim cristim commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

.pre-commit-config.yaml listed pre-push in default_stages but did not set default_install_hook_types. As a result, a plain pre-commit install wired only the pre-commit git hook, and the pre-push go-test stage never ran locally. This PR adds default_install_hook_types: [pre-commit, pre-push] and updates the go-test block's comment, which still told devs to pass --hook-type pre-push by hand.

Closes #10

Verification (local, macOS, at a4c7de3)

  • pre-commit validate-config .pre-commit-config.yaml passes (pre-commit 4.7.0).
  • Fail before, pass after, in a scratch repo running a plain pre-commit install:
    • With the config from origin/main, .git/hooks contains only pre-commit.
    • With this branch's config, it contains pre-commit and pre-push.
  • CI is unaffected. The workflow runs pre-commit run --all-files, which uses the default pre-commit stage; the new setting only affects install.
  • Side effect, which the issue intends: after a re-install, hooks in the default stages also run on git push.

Review

A non-author reviewer (Sonnet, high effort) did two passes on the diff and reproduced the install behaviour. No actionable findings. Its one nit, the stale --hook-type pre-push comment, is fixed in this PR. The final gate belongs to the merge driver.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated hook setup guidance to clarify that pre-commit install enables both pre-commit and pre-push checks, including Go tests and frontend builds and tests.

…h stages

default_stages already lists pre-push, but without
default_install_hook_types a plain `pre-commit install` wires only the
pre-commit git hook, so the pre-push go-test stage silently never ran
locally. Verified in a scratch repo: the old config installs only
pre-commit, the new one installs pre-commit and pre-push.

Closes #10

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

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 3b8f2147-9d95-4dc6-9f1b-1a9c87556c26
📥 Commits

Reviewing files that changed from the base of the PR and between 99ad252 and a4c7de3.

📒 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 hook types. Comments state that plain pre-commit install installs both hooks.

Changes: Hook installation

Layer / File(s) Summary
Set default hook types
.pre-commit-config.yaml
The configuration sets default_install_hook_types to [pre-commit, pre-push]. Updated comments state that plain pre-commit install installs both hooks.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to a4c7d

Plain installation is configured to arm both hook stages for compatible pre-commit versions. No supported-workflow incompatibility or CI regression is established, so no merge-blocking 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 configuration change: setting default_install_hook_types so a standard install enables both hook stages.
Linked Issues check Passed Issue #10 requires default_install_hook_types: [pre-commit, pre-push] and a valid pre-commit configuration. The whole-PR diff adds the exact setting to .pre-commit-config.yaml. The PR reports that…
Out of Scope Changes check Passed The PR changes only .pre-commit-config.yaml. The added comment explains the new installation behavior and supports issue #10. No unrelated implementation or behavior is present in the reviewed diff.
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…
✨ 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

Merge-driver verdict: CLEAN at a4c7de371d6f82e12b4c1a9b544ce763be3cf74c

An independent adversarial reviewer (Sonnet, high effort, cold brief, not the author) reviewed the full diff. It found no blocking findings.

  • Verified: every stages: value in .pre-commit-config.yaml is pre-push or comes from default_stages: [pre-commit, pre-push], and the new default_install_hook_types: [pre-commit, pre-push] covers both.
  • Local verification (real, macOS, pre-commit 4.7.0): I copied the PR's config into a throwaway git init repo and ran a plain pre-commit install. It installed both .git/hooks/pre-commit and .git/hooks/pre-push, and pre-commit validate-config passed.
  • Non-blocking nit: one reworded comment line near L252 runs long.
  • CI: all green at this SHA.
  • CodeRabbit: "No actionable comments".
  • Labels: match chore(pre-commit): set default_install_hook_types so install arms both stages [mcp part] #10.

@cristim
cristim merged commit 1d678c6 into main Oct 16, 2026
13 checks passed
@cristim
cristim deleted the chore/10-install-hook-types branch October 16, 2026 01:17
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 [mcp part]

1 participant