-
-
Notifications
You must be signed in to change notification settings - Fork 124
Warn when config file exists but fails to parse #1487
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds warnings when prek install encounters a configuration file that exists but fails to parse, preventing silent fallback to default behavior that could mask configuration errors.
Changes:
- Warning message now displays the specific parse error when config file is invalid
- Fallback to default hook types still occurs, but users are notified
- Test coverage added for invalid config warning behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/prek/tests/install.rs |
Adds integration test verifying warning appears for invalid config |
crates/prek/src/cli/install.rs |
Implements warning logic for config parse errors in two code paths |
📦 Cargo Bloat ComparisonBinary size change: +0.00% (22.5 MiB → 22.5 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1487 +/- ##
==========================================
+ Coverage 91.35% 91.51% +0.15%
==========================================
Files 87 87
Lines 18187 18167 -20
==========================================
+ Hits 16615 16625 +10
+ Misses 1572 1542 -30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Previously, `prek install` would silently fall back to default hook types when the config file existed but contained errors. Now it shows a warning with the specific error (e.g., "missing field `rev`"). Co-Authored-By: Claude Opus 4.5 <[email protected]>
6c3e430 to
fdb1e2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
|
Thanks! |
Summary
Previously,
prek installwould silently fall back to default hook types when the config file existed but contained errors (e.g., invalid YAML or missing required fields). This made it hard to notice configuration mistakes.Now it shows a warning with the specific error:
Test plan
install_invalid_config_warningtest🤖 Generated with Claude Code