|
| 1 | +name: Makefile Quality Check |
| 2 | + |
| 3 | +# ============================================================================ |
| 4 | +# WORKFLOW MAINTENANCE INSTRUCTIONS FOR FUTURE AGENTS/DEVELOPERS |
| 5 | +# ============================================================================ |
| 6 | +# |
| 7 | +# This workflow validates the Makefile for quality, consistency, and best practices. |
| 8 | +# It contains checks that reference specific Makefile content. When updating the |
| 9 | +# Makefile, you MUST keep this workflow in sync. |
| 10 | +# |
| 11 | +# WHEN TO UPDATE THIS WORKFLOW: |
| 12 | +# |
| 13 | +# 1. ADDING/REMOVING CORE TARGETS |
| 14 | +# - Location: "Verify all required targets exist" step (lines ~45-73) |
| 15 | +# - Action: Update the `required_targets` array to match critical Makefile targets |
| 16 | +# - Verify: Run `grep "^target-name:" Makefile` to confirm target exists |
| 17 | +# - Example: If you add a new core target like "test-integration", add it to the array |
| 18 | +# |
| 19 | +# 2. CHANGING HELP OUTPUT FORMAT |
| 20 | +# - Location: "Test help target output" step (lines ~129-150) |
| 21 | +# - Action: Update the grep patterns to match new section headers |
| 22 | +# - Verify: Run `make help | grep "Section Name:"` to see actual output |
| 23 | +# - Example: If you rename "Quick Start:" to "Getting Started:", update line ~135 |
| 24 | +# |
| 25 | +# 3. ADJUSTING QUALITY THRESHOLDS |
| 26 | +# - Location: "Verify target documentation" step (lines ~152-172) |
| 27 | +# - Action: Update percentage threshold (currently 50% minimum) |
| 28 | +# - Rationale: Threshold represents minimum acceptable documentation coverage |
| 29 | +# - Example: If requiring stricter docs, change line ~167 to higher percentage |
| 30 | +# |
| 31 | +# 4. CHANGING VARIABLE NAMES |
| 32 | +# - Location: "Check for hardcoded values" step (lines ~109-127) |
| 33 | +# - Action: Update search patterns if NAMESPACE or CONTAINER_ENGINE variable names change |
| 34 | +# - Verify: Ensure grep patterns exclude the new variable declaration line |
| 35 | +# - Example: If NAMESPACE becomes KUBE_NAMESPACE, update all references |
| 36 | +# |
| 37 | +# 5. ADDING NEW SCRIPT DIRECTORIES |
| 38 | +# - Location: "on.pull_request.paths" section (lines ~5-8) |
| 39 | +# - Action: Add new script paths that should trigger this workflow |
| 40 | +# - Example: If adding scripts/validation/*.sh, add to paths filter |
| 41 | +# |
| 42 | +# HOW TO VERIFY CHANGES: |
| 43 | +# |
| 44 | +# 1. Test locally before committing: |
| 45 | +# - Run: `make validate-makefile` (should pass) |
| 46 | +# - Run: `make lint-makefile` (should pass) |
| 47 | +# - Verify: All referenced targets exist in Makefile |
| 48 | +# - Verify: All help output strings match actual output |
| 49 | +# |
| 50 | +# 2. After updating required_targets array: |
| 51 | +# - Run: `for target in target1 target2; do grep -q "^${target}:" Makefile && echo "✓ $target" || echo "✗ MISSING: $target"; done` |
| 52 | +# |
| 53 | +# 3. After updating help output checks: |
| 54 | +# - Run: `make help > /tmp/test.txt && grep -E "Section Name:" /tmp/test.txt` |
| 55 | +# |
| 56 | +# IMPLEMENTATION PRINCIPLES: |
| 57 | +# |
| 58 | +# - NO MAGIC STRINGS: All strings checked must exist in actual Makefile output |
| 59 | +# - NO HALLUCINATIONS: All file paths must exist in repository |
| 60 | +# - DOCUMENTED THRESHOLDS: All numeric thresholds must have rationale comments |
| 61 | +# - FAIL-FAST: Hard requirements (missing targets) fail the workflow |
| 62 | +# - WARN-SOFT: Quality suggestions (documentation %) show warnings only |
| 63 | +# |
| 64 | +# ============================================================================ |
| 65 | + |
| 66 | +on: |
| 67 | + pull_request: |
| 68 | + paths: |
| 69 | + - 'Makefile' |
| 70 | + - '.github/workflows/makefile-quality.yml' |
| 71 | + push: |
| 72 | + branches: |
| 73 | + - main |
| 74 | + paths: |
| 75 | + - 'Makefile' |
| 76 | + |
| 77 | +jobs: |
| 78 | + validate-makefile: |
| 79 | + name: Validate Makefile |
| 80 | + runs-on: ubuntu-latest |
| 81 | + timeout-minutes: 5 |
| 82 | + |
| 83 | + steps: |
| 84 | + - name: Checkout code |
| 85 | + uses: actions/checkout@v5 |
| 86 | + |
| 87 | + - name: Install shellcheck |
| 88 | + run: | |
| 89 | + sudo apt-get update |
| 90 | + sudo apt-get install -y shellcheck |
| 91 | + shellcheck --version |
| 92 | + |
| 93 | + - name: Validate Makefile syntax |
| 94 | + run: | |
| 95 | + echo "🔍 Validating Makefile syntax and best practices..." |
| 96 | + make lint-makefile |
| 97 | + |
| 98 | + - name: Check shell scripts |
| 99 | + run: | |
| 100 | + echo "🔍 Checking embedded shell scripts..." |
| 101 | + make check-shell |
| 102 | + |
| 103 | + - name: Verify all required targets exist |
| 104 | + run: | |
| 105 | + echo "🔍 Verifying required targets exist..." |
| 106 | + |
| 107 | + # AGENT INSTRUCTIONS: This array contains the minimum set of targets that must exist |
| 108 | + # in the Makefile for the project to function correctly. Update this list when: |
| 109 | + # |
| 110 | + # ADD a target here if: It's a critical user-facing target that other automation depends on |
| 111 | + # REMOVE a target if: It's being deprecated AND no other workflows/docs reference it |
| 112 | + # VERIFY before changing: Search codebase for references to target name |
| 113 | + # |
| 114 | + # Current categories: |
| 115 | + # - Core: help (required for discoverability) |
| 116 | + # - Local dev: local-up, local-down, local-status, local-test-quick |
| 117 | + # - Quality: validate-makefile, lint-makefile, check-shell, makefile-health |
| 118 | + # - CI/CD: build-all, deploy, clean |
| 119 | + # |
| 120 | + # To verify a target exists: grep "^target-name:" Makefile |
| 121 | + required_targets=( |
| 122 | + "help" |
| 123 | + "local-up" |
| 124 | + "local-down" |
| 125 | + "local-status" |
| 126 | + "local-test-quick" |
| 127 | + "validate-makefile" |
| 128 | + "lint-makefile" |
| 129 | + "check-shell" |
| 130 | + "makefile-health" |
| 131 | + "build-all" |
| 132 | + "deploy" |
| 133 | + "clean" |
| 134 | + ) |
| 135 | + |
| 136 | + missing_targets=() |
| 137 | + for target in "${required_targets[@]}"; do |
| 138 | + if ! grep -q "^${target}:" Makefile; then |
| 139 | + missing_targets+=("$target") |
| 140 | + fi |
| 141 | + done |
| 142 | + |
| 143 | + if [ ${#missing_targets[@]} -gt 0 ]; then |
| 144 | + echo "❌ Missing required targets:" |
| 145 | + printf ' - %s\n' "${missing_targets[@]}" |
| 146 | + exit 1 |
| 147 | + else |
| 148 | + echo "✅ All required targets present" |
| 149 | + fi |
| 150 | + |
| 151 | + - name: Verify .PHONY declarations |
| 152 | + run: | |
| 153 | + echo "🔍 Verifying .PHONY declarations..." |
| 154 | + |
| 155 | + # Extract all target names (excluding internal targets starting with _) |
| 156 | + targets=$(grep -E '^[a-zA-Z][a-zA-Z0-9_-]+:' Makefile | grep -v '^_' | cut -d: -f1 | sort -u) |
| 157 | + |
| 158 | + # Extract .PHONY declarations |
| 159 | + phony_targets=$(grep '^\.PHONY:' Makefile | sed 's/^\.PHONY: //' | tr ' ' '\n' | sort -u) |
| 160 | + |
| 161 | + # Count targets and phony declarations |
| 162 | + target_count=$(echo "$targets" | wc -l) |
| 163 | + phony_count=$(echo "$phony_targets" | wc -l) |
| 164 | + |
| 165 | + echo "📊 Found $target_count targets, $phony_count in .PHONY" |
| 166 | + |
| 167 | + # Find targets not in .PHONY (excluding pattern rules and variable assignments) |
| 168 | + not_phony=() |
| 169 | + while IFS= read -r target; do |
| 170 | + if ! echo "$phony_targets" | grep -q "^${target}$"; then |
| 171 | + # Skip if it's a pattern rule or special target |
| 172 | + if [[ ! "$target" =~ ^% ]] && [[ ! "$target" =~ ^\. ]]; then |
| 173 | + not_phony+=("$target") |
| 174 | + fi |
| 175 | + fi |
| 176 | + done <<< "$targets" |
| 177 | + |
| 178 | + if [ ${#not_phony[@]} -gt 0 ]; then |
| 179 | + echo "⚠️ Targets not declared in .PHONY (may be intentional for file targets):" |
| 180 | + printf ' - %s\n' "${not_phony[@]}" | head -10 |
| 181 | + else |
| 182 | + echo "✅ All non-file targets properly declared in .PHONY" |
| 183 | + fi |
| 184 | + |
| 185 | + - name: Check for hardcoded values |
| 186 | + run: | |
| 187 | + echo "🔍 Checking for hardcoded values that should be variables..." |
| 188 | + |
| 189 | + # AGENT INSTRUCTIONS: This check detects hardcoded values that should use variables. |
| 190 | + # It prevents configuration drift and ensures Makefile remains configurable. |
| 191 | + # |
| 192 | + # WHEN TO UPDATE THESE CHECKS: |
| 193 | + # |
| 194 | + # 1. IF VARIABLE NAMES CHANGE (e.g., NAMESPACE → KUBE_NAMESPACE): |
| 195 | + # - Update the variable name in grep commands |
| 196 | + # - Update exclusion patterns (grep -v lines) |
| 197 | + # - Test: Ensure variable declaration line is excluded from warnings |
| 198 | + # |
| 199 | + # 2. IF DEFAULT VALUES CHANGE (e.g., ambient-code → new-namespace): |
| 200 | + # - Update the search string to match new default value |
| 201 | + # - Keep the variable name check pattern |
| 202 | + # - Current defaults defined in Makefile lines ~7-11 |
| 203 | + # |
| 204 | + # 3. IF ADDING NEW CONFIGURABLE VALUES: |
| 205 | + # - Add similar check block following this pattern |
| 206 | + # - Exclude: variable declarations, comments, help text |
| 207 | + # - Example: Check for hardcoded registry names |
| 208 | + # |
| 209 | + # CURRENT CHECKS: |
| 210 | + # - namespace: "ambient-code" should use $(NAMESPACE) variable |
| 211 | + # - container engine: "docker" or "podman" should use $(CONTAINER_ENGINE) variable |
| 212 | + # |
| 213 | + # WHY THESE EXCLUSIONS: |
| 214 | + # - "NAMESPACE ?=" = variable declaration itself (Makefile line 10) |
| 215 | + # - "^[0-9]*:#" = comments (documentation, not code) |
| 216 | + # - "@echo" = help text displayed to users (intentionally literal) |
| 217 | + # |
| 218 | + # TO VERIFY: grep -n "value" Makefile | grep -v "VARIABLE ?=" | grep -v "^[0-9]*:#" |
| 219 | + |
| 220 | + # Check for hardcoded namespaces outside of variable declaration (should use $(NAMESPACE)) |
| 221 | + # Excludes: NAMESPACE ?= line, comments, and @echo help text |
| 222 | + if grep -n "ambient-code" Makefile | grep -v "NAMESPACE ?=" | grep -v "^[0-9]*:#" | grep -v "@echo" | grep -q .; then |
| 223 | + echo "⚠️ Found hardcoded 'ambient-code' references (should use \$(NAMESPACE) variable):" |
| 224 | + grep -n "ambient-code" Makefile | grep -v "NAMESPACE ?=" | grep -v "^[0-9]*:#" | grep -v "@echo" | head -5 |
| 225 | + echo " To fix: Replace 'ambient-code' with '\$(NAMESPACE)' in these locations" |
| 226 | + fi |
| 227 | + |
| 228 | + # Check for hardcoded container engines outside of variable references |
| 229 | + # Excludes: lines with CONTAINER_ENGINE variable, comments, and help text |
| 230 | + if grep -nE "docker|podman" Makefile | grep -v "CONTAINER_ENGINE" | grep -v "^[0-9]*:#" | grep -v "@echo" | grep -q .; then |
| 231 | + echo "⚠️ Found hardcoded docker/podman references (should use \$(CONTAINER_ENGINE) variable):" |
| 232 | + grep -nE "docker|podman" Makefile | grep -v "CONTAINER_ENGINE" | grep -v "^[0-9]*:#" | grep -v "@echo" | head -5 |
| 233 | + echo " To fix: Replace 'docker' or 'podman' with '\$(CONTAINER_ENGINE)' in these locations" |
| 234 | + fi |
| 235 | + |
| 236 | + echo "✅ No problematic hardcoded values found" |
| 237 | + |
| 238 | + - name: Test help target output |
| 239 | + run: | |
| 240 | + echo "🔍 Testing help target..." |
| 241 | + make help > /tmp/help-output.txt |
| 242 | + |
| 243 | + # AGENT INSTRUCTIONS: These strings MUST match the exact section headers in the Makefile |
| 244 | + # help target output. When modifying the Makefile help format: |
| 245 | + # |
| 246 | + # 1. Run: make help | grep -E "Section Name:" to see actual output |
| 247 | + # 2. Update the grep patterns below to match new header text exactly |
| 248 | + # 3. Consider if renaming improves user experience (prefer clarity over brevity) |
| 249 | + # 4. These checks ensure help output meets minimum usability standards |
| 250 | + # |
| 251 | + # Current required sections defined in Makefile (lines ~39-49): |
| 252 | + # - "Quick Start:" - Essential commands for new users |
| 253 | + # - "Quality Assurance:" - Quality/validation commands |
| 254 | + # - "Available Targets:" - Complete target listing (auto-generated by awk) |
| 255 | + # |
| 256 | + # To verify: make help | grep -o "^[A-Za-z ]*:" | sort -u |
| 257 | + |
| 258 | + if ! grep -q "Quick Start:" /tmp/help-output.txt; then |
| 259 | + echo "❌ Help output missing 'Quick Start' section" |
| 260 | + echo " Update this check if you renamed the section in Makefile" |
| 261 | + exit 1 |
| 262 | + fi |
| 263 | + |
| 264 | + if ! grep -q "Quality Assurance:" /tmp/help-output.txt; then |
| 265 | + echo "❌ Help output missing 'Quality Assurance' section" |
| 266 | + echo " Update this check if you renamed the section in Makefile" |
| 267 | + exit 1 |
| 268 | + fi |
| 269 | + |
| 270 | + if ! grep -q "Available Targets:" /tmp/help-output.txt; then |
| 271 | + echo "❌ Help output missing 'Available Targets' section" |
| 272 | + echo " This is auto-generated by awk in Makefile line ~49" |
| 273 | + exit 1 |
| 274 | + fi |
| 275 | + |
| 276 | + echo "✅ Help target produces expected output" |
| 277 | + |
| 278 | + - name: Verify target documentation |
| 279 | + run: | |
| 280 | + echo "🔍 Checking target documentation..." |
| 281 | + |
| 282 | + # AGENT INSTRUCTIONS: This check measures documentation quality by counting |
| 283 | + # how many Makefile targets have inline help text (## comments). |
| 284 | + # |
| 285 | + # DOCUMENTATION FORMAT: target-name: ## Help text shown in 'make help' |
| 286 | + # |
| 287 | + # WHEN TO ADJUST THRESHOLD: |
| 288 | + # - 50% minimum = Current threshold for acceptable quality |
| 289 | + # - Increase to 75%+ if requiring comprehensive documentation |
| 290 | + # - Decrease to 30%+ only if many internal/experimental targets exist |
| 291 | + # |
| 292 | + # RATIONALE FOR 50% THRESHOLD: |
| 293 | + # - All user-facing targets SHOULD have documentation |
| 294 | + # - Internal targets (prefixed with _) are automatically excluded |
| 295 | + # - Helper targets may not need docs if only called by other targets |
| 296 | + # - 50% ensures at least half of public API is documented |
| 297 | + # |
| 298 | + # TO ADD DOCUMENTATION: Add ## comment after target definition |
| 299 | + # Example: my-target: ## Description of what this target does |
| 300 | + # |
| 301 | + # TO VERIFY: grep '^target-name:.*##' Makefile |
| 302 | + |
| 303 | + # Count targets with documentation (excluding internal targets starting with _) |
| 304 | + total_targets=$(grep -E '^[a-zA-Z][a-zA-Z0-9_-]+:' Makefile | grep -v '^_' | wc -l) |
| 305 | + documented_targets=$(grep -E '^[a-zA-Z][a-zA-Z0-9_-]+:.*##' Makefile | wc -l) |
| 306 | + |
| 307 | + echo "📊 $documented_targets/$total_targets targets have help text" |
| 308 | + |
| 309 | + # Calculate percentage (threshold: 50% minimum for good documentation practice) |
| 310 | + if [ "$total_targets" -gt 0 ]; then |
| 311 | + percentage=$((documented_targets * 100 / total_targets)) |
| 312 | + echo "📈 Documentation coverage: ${percentage}%" |
| 313 | + |
| 314 | + if [ "$percentage" -lt 50 ]; then |
| 315 | + echo "⚠️ Documentation coverage below 50%, consider adding help text (## comments) to more targets" |
| 316 | + echo " Add documentation by appending '## Description' after target definition" |
| 317 | + else |
| 318 | + echo "✅ Good target documentation coverage (${percentage}%)" |
| 319 | + fi |
| 320 | + fi |
| 321 | + |
| 322 | + - name: Run comprehensive validation |
| 323 | + run: | |
| 324 | + echo "🔍 Running comprehensive Makefile validation..." |
| 325 | + make validate-makefile |
| 326 | + |
| 327 | + - name: Summary |
| 328 | + if: success() |
| 329 | + run: | |
| 330 | + echo "" |
| 331 | + echo "═══════════════════════════════════════" |
| 332 | + echo " Makefile Quality Check Summary" |
| 333 | + echo "═══════════════════════════════════════" |
| 334 | + echo "" |
| 335 | + echo "✅ All quality checks passed!" |
| 336 | + echo "" |
| 337 | + echo "The Makefile meets quality standards and is ready for use." |
| 338 | +
|
0 commit comments