🔧 Improve pre-push error message for PR size violations #81
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enhanced error message with clear visual separation and actionable guidance:
Before (unclear):
PR too large (644 > 600 lines). Please split into smaller slices.
[followed by Git's generic 'error: failed to push' message]
After (crystal clear):
═══════════════════════════════════════════════════════════════
❌ PRE-PUSH CHECK FAILED: PR TOO LARGE
═══════════════════════════════════════════════════════════════
Your changes: 644 lines (636 insertions, 8 deletions)
Maximum allowed: 600 lines per PR
Action required: Split changes into smaller, focused PRs
💡 Available options:
1. Split PR: Recommended approach
2. Override check: touch .preflight-allow-large-pr
3. Bypass hook: git push --no-verify (not recommended)
═══════════════════════════════════════════════════════════════
Push aborted. Fix the issue above and try again.
═══════════════════════════════════════════════════════════════
Changes:
- Added visual separators (borders) for attention
- Show exact breakdown (insertions + deletions)
- Numbered options for clarity
- Clear "Push aborted" message before Git's error
- Inserted blank lines for better readability
Fixes #80
💡 Tip: Consider Using Draft PRsBenefits of opening PRs as drafts initially:
How to convert:
This is just a friendly reminder - feel free to continue as is! 😊 |
kevalyq
added a commit
to SecPal/frontend
that referenced
this pull request
Nov 2, 2025
Same fix as SecPal/api#81 - enhanced error message clarity with: - Visual separators for attention - Exact breakdown (insertions + deletions) - Numbered options for bypass - Clear "Push aborted" message See SecPal/api#80 for problem description
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the preflight script's error messaging for oversized PRs by improving the calculation methodology and user feedback. The changes make the error message more informative and actionable.
- Refactored line count calculation to separately track insertions and deletions before summing
- Enhanced error message with detailed formatting, clear statistics breakdown, and structured action options
kevalyq
added a commit
to SecPal/contracts
that referenced
this pull request
Nov 2, 2025
Same fix as SecPal/api#81 - enhanced error message clarity with: - Visual separators for attention - Exact breakdown (insertions + deletions) - Numbered options for bypass - Clear "Push aborted" message See SecPal/api#80 for problem description
kevalyq
added a commit
to SecPal/.github
that referenced
this pull request
Nov 2, 2025
Same fix as SecPal/api#81 - enhanced error message clarity with: - Visual separators for attention - Exact breakdown (insertions + deletions) - Numbered options for bypass - Clear "Push aborted" message See SecPal/api#80 for problem description
This was referenced Nov 2, 2025
💡 Tip: Consider Using Draft PRsBenefits of opening PRs as drafts initially:
How to convert:
This is just a friendly reminder - feel free to continue as is! 😊 |
kevalyq
added a commit
to SecPal/contracts
that referenced
this pull request
Nov 2, 2025
* feat: add CHANGELOG validation to preflight script SPDX-FileCopyrightText: 2025 SecPal SPDX-License-Identifier: MIT Syncs CHANGELOG validation logic from api repo (PR #77) to contracts. Changes: - Validates [Unreleased] section exists on feature/fix/refactor branches - Checks for minimum content (MIN_CHANGELOG_LINES=3) - Exempts docs/*, chore/*, ci/*, test/* branches - Robust parsing handles [Unreleased] as last section (edge case fix) - Uses configurable variables for maintainability Context: Multi-repo script synchronization initiative. Ensures consistent CHANGELOG policy enforcement across all SecPal repos. Benefits: - ✅ Prevents "forgot to update CHANGELOG" PR comments - ✅ Catches empty [Unreleased] sections (copy-paste artifacts) - ✅ Smart exemptions for docs-only branches - ✅ Early feedback (local preflight vs CI failure) Related: - api #77: Original CHANGELOG validation implementation - frontend #55: Parallel sync to frontend repo - .github #168: Template documentation * fix: address Copilot feedback (case statement, grep -nE, HTML patterns, MIN_CHANGELOG_LINES) - Replace bash-specific [[ =~ ]] with POSIX case statement for portability - Use grep -nE for robust [Unreleased] matching (supports Keep a Changelog links) - Extract duplicated grep chains to filter_changelog_content() with whitespace tolerance - Clarify MIN_CHANGELOG_LINES comment about what is counted (substantive content only) Addresses all 4 Copilot review comments on PR #39 * fix: improve pre-push error message for PR size violations Same fix as SecPal/api#81 - enhanced error message clarity with: - Visual separators for attention - Exact breakdown (insertions + deletions) - Numbered options for bypass - Clear "Push aborted" message See SecPal/api#80 for problem description * fix: remove --no-verify suggestion per Copilot review * refactor: optimize CHANGELOG validation per Copilot review - Combine multiple grep -Ev calls into single grep with pattern group - Use grep -m 1 for early termination (performance improvement) - Fix sed range calculation (UNRELEASED_END is relative, not absolute) - Add clarifying comments about CHANGELOG_EXEMPT_PREFIXES and case sync - Improve warning message with specific line count and non-blocking note Addresses Copilot review comments in PR #40
kevalyq
added a commit
to SecPal/frontend
that referenced
this pull request
Nov 2, 2025
* feat: add CHANGELOG validation to preflight script SPDX-FileCopyrightText: 2025 SecPal SPDX-License-Identifier: MIT Syncs CHANGELOG validation logic from api repo (PR #77) to frontend. Changes: - Validates [Unreleased] section exists on feature/fix/refactor branches - Checks for minimum content (MIN_CHANGELOG_LINES=3) - Exempts docs/*, chore/*, ci/*, test/* branches - Robust parsing handles [Unreleased] as last section (edge case fix) - Uses configurable variables for maintainability Context: Multi-repo script synchronization initiative. Ensures consistent CHANGELOG policy enforcement across all SecPal repos. Benefits: - ✅ Prevents "forgot to update CHANGELOG" PR comments - ✅ Catches empty [Unreleased] sections (copy-paste artifacts) - ✅ Smart exemptions for docs-only branches - ✅ Early feedback (local preflight vs CI failure) Related: - api #77: Original CHANGELOG validation implementation - .github #168: Template documentation * fix: address Copilot feedback (POSIX compliance, deduplication, HTML patterns) - Replace bash-specific [[ =~ ]] with POSIX grep -qE for portability - Extract duplicated grep filter chains to filter_changelog_content() function - Make HTML comment patterns whitespace-tolerant with grep -Ev and [[:space:]]* Addresses Copilot review comments on PR #55 * fix: improve pre-push error message for PR size violations Same fix as SecPal/api#81 - enhanced error message clarity with: - Visual separators for attention - Exact breakdown (insertions + deletions) - Numbered options for bypass - Clear "Push aborted" message See SecPal/api#80 for problem description * fix: remove --no-verify suggestion per Copilot review * refactor: optimize CHANGELOG validation per Copilot review - Combine multiple grep -Ev calls into single grep with pattern group - Use grep -m 1 for early termination (performance improvement) - Add clarifying comment about regex anchor usage in branch matching - Improve warning message with specific line count threshold - Add actionable guidance for CHANGELOG format requirements Addresses Copilot review comments in PR #56
kevalyq
added a commit
to SecPal/.github
that referenced
this pull request
Nov 2, 2025
* docs: add mail system patterns to copilot config - Add mail: section to copilot-config.yaml - Document Mailpit integration with DDEV - Add Mailable patterns and examples - Add testing patterns with Mail::fake() - Include security rules for email handling - Update copilot-instructions.md with mail section Related: SecPal/api#78 (Production Test Phase 2) * style: fix markdown linting * refactor: remove API-specific code examples from instructions - Removed entire Mail System section (60+ lines of code examples) - Shortened Data Protection section (removed code, kept rules only) - Instructions should contain RULES, not code documentation - Mail System is API-specific, not workspace-wide Impact: - copilot-config.yaml: -82 lines - copilot-instructions.md: -123 lines Total: -205 lines of bloat removed Rationale: Code examples belong in api/docs/, not .github/ Related: api/docs/MAIL_SYSTEM.md created for detailed examples * fix: improve pre-push error message for PR size violations Same fix as SecPal/api#81 - enhanced error message clarity with: - Visual separators for attention - Exact breakdown (insertions + deletions) - Numbered options for bypass - Clear "Push aborted" message See SecPal/api#80 for problem description * fix: remove --no-verify suggestion per Copilot review Removed option 3 as it violates Critical Rule #4: No Bypass policy.
kevalyq
added a commit
that referenced
this pull request
Nov 2, 2025
* fix: remove --no-verify suggestion from error message Removed option 3 from pre-push error message as it violates Critical Rule #4: No Bypass policy. The only legitimate override is .preflight-allow-large-pr for exceptional cases with maintainer approval. Follow-up to #81 based on Copilot review feedback. * refactor: optimize CHANGELOG validation for consistency - Replace bash-specific [[ =~ ]] with POSIX-compliant case statement - Combine multiple grep -v calls into single grep -Ev with pattern group - Use grep -m 1 for early termination (performance improvement) - Fix sed range calculation (UNRELEASED_END is relative, not absolute) - Add clarifying comments about CHANGELOG_EXEMPT_PREFIXES and case sync - Improve warning message with specific line count and non-blocking note - Extract filter logic to reusable filter_changelog_content() function Aligns api/scripts/preflight.sh with optimizations applied to frontend, contracts, and .github repositories per Copilot review feedback.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When pushing changes that exceed the 600-line limit, the pre-push hook showed an unclear error:
Users couldn't easily see:
Solution
Enhanced error message with clear visual structure:
Changes
Testing
Manually tested error output format. The message now appears before Git's generic error, making the cause immediately clear.
Fixes #80