Skip to content

fix(hooks): honor pre-commit filenames in values-check & avoid set -e abort#63

Merged
michaelbeutler merged 1 commit into
mainfrom
fix/values-check-hook-set-e
May 20, 2026
Merged

fix(hooks): honor pre-commit filenames in values-check & avoid set -e abort#63
michaelbeutler merged 1 commit into
mainfrom
fix/values-check-hook-set-e

Conversation

@michaelbeutler
Copy link
Copy Markdown
Contributor

@michaelbeutler michaelbeutler commented May 20, 2026

values-check.sh read git diff --cached --name-only and, under set -e, the assignment aborted the script the moment grep matched nothing (e.g. when run via pre-commit run --all-files with nothing staged) — printing only the header and exiting 1.

pre-commit already passes the matched values.yaml paths as arguments (pass_filenames defaults to true), so use "$@" when present and fall back to staged files otherwise. Add || true so an empty match no longer trips set -e. Behaviour verified for: args passed, staged-only, clean tree (graceful skip), and invalid YAML (still rejected).

Summary by CodeRabbit

  • Chores
    • Enhanced the values validation pre-commit hook with improved file filtering logic and error handling.

Review Change Stack

… abort

values-check.sh read `git diff --cached --name-only` and, under `set -e`,
the assignment aborted the script the moment grep matched nothing (e.g. when
run via `pre-commit run --all-files` with nothing staged) — printing only the
header and exiting 1.

pre-commit already passes the matched values.yaml paths as arguments
(pass_filenames defaults to true), so use "$@" when present and fall back to
staged files otherwise. Add `|| true` so an empty match no longer trips
`set -e`. Behaviour verified for: args passed, staged-only, clean tree
(graceful skip), and invalid YAML (still rejected).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 848121d7-6729-43f3-a407-d386fe7199aa

📥 Commits

Reviewing files that changed from the base of the PR and between 648f3e0 and 74d141d.

📒 Files selected for processing (1)
  • scripts/hooks/values-check.sh

📝 Walkthrough

Walkthrough

The values-check.sh pre-commit hook now accepts file arguments and uses them to determine which values.yaml files to validate. When arguments are provided, they are filtered for paths ending in values.yaml. When no arguments exist, the hook falls back to the previous behavior of using git diff --cached --name-only. Both paths include || true to prevent set -e from aborting on empty grep results.

Changes

Pre-commit hook argument handling

Layer / File(s) Summary
Argument-aware file selection logic
scripts/hooks/values-check.sh
The changed_values assignment detects provided file arguments and filters them for values.yaml paths, or falls back to git diff --cached --name-only when no arguments exist. Both branches use || true to suppress errors when grep finds no matches under set -e.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: fixing the hook to honor pre-commit filenames and avoiding set -e abort issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/values-check-hook-set-e

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

@michaelbeutler michaelbeutler merged commit 99be2e7 into main May 20, 2026
11 of 12 checks passed
@michaelbeutler michaelbeutler deleted the fix/values-check-hook-set-e branch May 20, 2026 10:42
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.

1 participant