Skip to content

Commit 1ddc5bc

Browse files
committed
feat: refactor git hooks with common library for improved logging and organization
1 parent 83392f6 commit 1ddc5bc

File tree

6 files changed

+600
-281
lines changed

6 files changed

+600
-281
lines changed
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Git Hooks Refactoring Summary
2+
3+
## Overview
4+
Successfully refactored the git hooks system to extract common functionality and improve code organization with consistent colored logging.
5+
6+
## Changes Made
7+
8+
### 1. Created Common Library (`/booster/tools/git-hooks/lib/common.sh`)
9+
10+
**Features:**
11+
- **Colored Logging System**: Consistent, emoji-enhanced logging functions
12+
- `log_info()` - Information messages (cyan with ℹ️)
13+
- `log_success()` - Success messages (green with ✅)
14+
- `log_warning()` - Warning messages (yellow with ⚠️)
15+
- `log_error()` - Error messages (red with ❌)
16+
- `log_step()` - Process steps (blue with 📋)
17+
- `log_tool()` - Tool execution (cyan with 🔧)
18+
- `log_check()` - Check operations (blue with 🔍)
19+
- `log_skip()` - Skipped operations (yellow with 🚫)
20+
- `log_celebrate()` - Success celebration (green with 🎉)
21+
22+
- **Environment Setup**: Standardized git environment detection
23+
- `setup_git_environment()` - Detects ROOT, BASE, GIT_DIR, RUNNER
24+
- `setup_branch_info()` - Gets current branch name
25+
- `setup_common_paths()` - Sets up common file paths
26+
27+
- **Early Exit Conditions**: Reusable skip logic
28+
- `is_ci_environment()` - CI detection
29+
- `is_merge_in_progress()` - Merge state detection
30+
- `should_skip_in_ci()` - CI skip with logging
31+
- `should_skip_during_merge()` - Merge skip with logging
32+
33+
- **Dependency Checks**: Common validation functions
34+
- `check_runner_available()` - Validates runner script
35+
- `check_node_modules()` - Validates Node.js dependencies
36+
- `check_vendor_directory()` - Validates PHP dependencies
37+
- `check_file_exists()` - Generic file validation
38+
39+
- **Command Execution**: Standardized command running
40+
- `run_command()` - Execute with logging
41+
- `run_command_quiet()` - Execute silently
42+
43+
- **Package Detection**: Composer package management
44+
- `has_composer_package()` - Check for Composer packages
45+
- `has_tool()` - Check for vendor tools
46+
- Automatic cache cleanup on exit
47+
48+
### 2. Refactored `setup.sh`
49+
50+
**Before:** Manual color definitions, basic echo statements
51+
**After:**
52+
- Uses common library for all logging
53+
- Consistent colored output with emojis
54+
- Cleaner error handling with `error_exit()`
55+
- Simplified function names (e.g., `check_runner_available()``check_runner_script()`)
56+
57+
### 3. Refactored `commit-msg.bash`
58+
59+
**Before:** Duplicate environment setup, basic error messages
60+
**After:**
61+
- Uses common library for environment setup
62+
- Consistent logging throughout
63+
- Simplified validation functions
64+
- Better error reporting with `error_exit()`
65+
66+
### 4. Refactored `pre-commit.bash`
67+
68+
**Before:** Inline emojis, manual PHP file filtering, repetitive echo statements
69+
**After:**
70+
- Uses common library functions
71+
- Clean separation of concerns with dedicated functions:
72+
- `check_staged_php_files()` - File detection
73+
- `run_php_syntax_check()` - Syntax validation
74+
- `run_rector_fix()` - Rector execution
75+
- `run_ecs_fix()` - ECS execution
76+
- `run_deptrac_check()` - Architecture validation
77+
- `run_phpstan_analysis()` - Static analysis
78+
- `run_psalm_analysis()` - Psalm analysis
79+
- Consistent tool detection with `has_tool()`
80+
81+
### 5. Refactored `pre-push.bash`
82+
83+
**Before:** Duplicate utility functions, basic echo statements
84+
**After:**
85+
- Uses common library for all shared functionality
86+
- Removed duplicate composer cache logic (handled by common.sh)
87+
- Consistent logging and error handling
88+
- Cleaner function organization
89+
90+
## Benefits Achieved
91+
92+
### Code Quality Improvements
93+
- **DRY Principle**: Eliminated code duplication across all hooks
94+
- **Separation of Concerns**: Common functionality extracted to library
95+
- **Consistent Interface**: All hooks now use same logging and error handling
96+
97+
### User Experience Improvements
98+
- **Visual Consistency**: All hooks now use consistent colored output with emojis
99+
- **Better Error Messages**: Clear, actionable error messages with consistent formatting
100+
- **Progress Indication**: Clear step-by-step progress reporting
101+
102+
### Maintainability Improvements
103+
- **Single Source of Truth**: Common functionality in one place
104+
- **Easier Updates**: Changes to logging/environment setup affect all hooks
105+
- **Better Testing**: Common functions can be tested independently
106+
107+
## Validation
108+
All refactored scripts pass bash syntax validation:
109+
-`lib/common.sh` - Syntax valid
110+
-`setup.sh` - Syntax valid
111+
-`commit-msg.bash` - Syntax valid
112+
-`pre-commit.bash` - Syntax valid
113+
-`pre-push.bash` - Syntax valid
114+
115+
## Future Improvements
116+
- Consider adding configuration file support for hook behavior
117+
- Add unit tests for common library functions
118+
- Consider adding hook-specific configuration options
119+
- Add support for custom emoji/color schemes

booster/tools/git-hooks/hooks/commit-msg.bash

Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,93 +4,55 @@
44
set -euo pipefail
55
IFS=$'\n\t'
66

7-
# --- Environment Setup ---
8-
setup_environment() {
9-
ROOT=$(git rev-parse --show-toplevel)
10-
if [ -f "$ROOT/booster/tools/runner.sh" ]; then
11-
BASE="$ROOT/booster"
12-
else
13-
BASE="$ROOT"
14-
fi
15-
readonly ROOT BASE
16-
17-
local git_dir
18-
git_dir=$(git rev-parse --git-dir)
19-
readonly GIT_DIR="$git_dir"
20-
readonly RUNNER="$BASE/tools/runner.sh"
21-
22-
local current_branch
23-
current_branch=$(git symbolic-ref --short HEAD)
24-
readonly CURRENT_BRANCH="$current_branch"
25-
26-
# Use relative path for DDEV compatibility
27-
readonly UTIL_SCRIPT="./tools/commit-utils.js"
28-
}
7+
# Source common library
8+
PROJECT_ROOT="$(git rev-parse --show-toplevel)"
9+
source "$PROJECT_ROOT/tools/git-hooks/lib/common.sh"
2910

3011
# --- Early Exit Conditions ---
31-
should_skip_hook() {
32-
# Skip during merge
33-
if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
34-
echo "Merge in progress. Skipping commit message checks."
12+
should_skip_commit_checks() {
13+
if should_skip_during_merge; then
3514
return 0
3615
fi
37-
3816
return 1
3917
}
4018

4119
check_dependencies() {
42-
if [ ! -d "$BASE/node_modules" ]; then
43-
local install_cmd
44-
if [ -d "$ROOT/.ddev" ]; then
45-
install_cmd="ddev pnpm install"
46-
else
47-
install_cmd="pnpm install"
48-
fi
49-
echo "ERROR: No node_modules directory detected. Please run '$install_cmd' before committing." >&2
50-
exit 1
51-
fi
52-
}
53-
54-
error() {
55-
echo "ERROR: $*" >&2
56-
exit 1
20+
check_node_modules
5721
}
5822

5923
# --- Validation Functions ---
6024
validate_branch_name() {
61-
if ! bash "$RUNNER" node_modules/.bin/validate-branch-name -t "$CURRENT_BRANCH" > /dev/null 2>&1; then
25+
if ! run_command_quiet node_modules/.bin/validate-branch-name -t "$CURRENT_BRANCH"; then
6226
# Run again without suppression to show the error details
6327
bash "$RUNNER" node_modules/.bin/validate-branch-name -t "$CURRENT_BRANCH"
64-
error "Branch name validation failed. See rules in validate-branch-name.config.cjs."
28+
error_exit "Branch name validation failed. See rules in validate-branch-name.config.cjs."
6529
fi
6630
}
6731

6832
lint_commit_message() {
6933
local commit_file="$1"
70-
bash "$RUNNER" node_modules/.bin/commitlint --edit "$commit_file"
34+
run_command "commitlint" node_modules/.bin/commitlint --edit "$commit_file"
7135
}
7236

7337
append_ticket_footer() {
7438
local commit_file="$1"
7539

76-
if [ ! -f "$UTIL_SCRIPT" ]; then
77-
error "Missing commit-utils.js helper script."
78-
fi
40+
check_file_exists "$UTIL_SCRIPT" "commit-utils.js helper script"
7941

8042
# Query NEED_TICKET & FOOTER_LABEL via helper script
8143
local need_ticket footer_label
8244
if ! need_ticket=$(bash "$RUNNER" node "$UTIL_SCRIPT" --need-ticket 2>/dev/null); then
83-
error "Failed to determine ticket requirement."
45+
error_exit "Failed to determine ticket requirement."
8446
fi
8547
if ! footer_label=$(bash "$RUNNER" node "$UTIL_SCRIPT" --footer-label 2>/dev/null); then
86-
error "Failed to determine footer label."
48+
error_exit "Failed to determine footer label."
8749
fi
8850

8951
if [ "${need_ticket}" = "yes" ]; then
9052
local ticket_id
9153
ticket_id=$(bash "$RUNNER" node "$UTIL_SCRIPT" --extract-ticket "$CURRENT_BRANCH" 2>/dev/null || true)
9254
if [ -z "${ticket_id:-}" ]; then
93-
error "No ticket ID found in branch name."
55+
error_exit "No ticket ID found in branch name."
9456
fi
9557

9658
local commit_body
@@ -105,9 +67,9 @@ append_ticket_footer() {
10567
main() {
10668
local commit_file="$1"
10769

108-
setup_environment
70+
init_common
10971

110-
if should_skip_hook; then
72+
if should_skip_commit_checks; then
11173
exit 0
11274
fi
11375

0 commit comments

Comments
 (0)