6.4 KiB
Code Review: PR #55 – Fix #54: Pre-Commit Hook als Gate für jeden Commit
Reviewer: Claw (AI Code Reviewer) Datum: 2026-05-22 Branch: feature/issue-54-precommit-lint-gate → main Betreffene Issues: #53 (PR-Gate / Branch Protection), #54 (Pre-Commit Hook als Gate)
Zusammenfassung
PR #55 integriert Lint-Checks (PHP, CSS, HTML) als Gate in den CI-Deploy-Workflow und ergänzt PHP-Lint im Pre-Commit Hook. Zudem werden Helper-Scripts für AI-Agent-Commits bereitgestellt.
Geänderte Dateien
| Datei | Änderung |
|---|---|
.gitea/workflows/deploy-test.yml |
3 neue Lint-Jobs (PHP, CSS, HTML) + needs-Abhängigkeit für Deploy |
AGENTS.md |
Pre-Commit-Hook-Dokumentation, OS-Korrektur Windows→Linux |
package.json |
lint:php-Script + lint-staged Eintrag für .php-Dateien |
scripts/lint-php.sh |
Neu – Pre-Commit PHP Syntax-Check |
scripts/safe-commit.sh |
Neu – Commit-Wrapper mit garantiertem Lint-Run |
Code-Quality-Checkliste
| Kriterium | Bewertung | Anmerkung |
|---|---|---|
| Funktionslänge | ✅ Gut | Alle Funktionen/Scripts sind kurz und fokussiert |
| Dateilänge | ✅ Gut | Keine überlangen Dateien |
| Error-Handling | ✅ Gut | set -euo pipefail in Scripts, Exit-Codes korrekt |
| Input-Validation | ✅ Gut | safe-commit.sh prüft ob Argument vorhanden |
| Secrets | ✅ Keine | Keine Secrets im Code |
| Namensgebung | ✅ Klar | lint-php.sh, safe-commit.sh – selbsterklärend |
| DRY | ⚠️ Minor | lint:php in package.json dupliziert Logik aus lint-php.sh (anderer Ansatz, aber gleicher Zweck) |
| Konsistenz | ✅ Gut | Shell-Scripts folgen bash-idiomatischem Stil |
Sicherheitsprüfung
- ✅ Keine Secrets, Tokens oder Credentials im Code
- ✅ Keine unsichere
eval/exec-Konstrukte - ✅
set -euo pipefailin allen Scripts - ✅ Keine Shell-Injection-Vektoren (File-Argumente korrekt gequotet)
Architekturkonformität
- ✅ Lint-Jobs korrekt als separate Jobs im CI-Workflow
- ✅ Deploy-Job hat
needs: [lint-php, lint-css, lint-html]→ Deploy nur bei grünen Lints - ✅ Pre-Commit Hook (
.husky/pre-commit→npx lint-staged) bleibt intakt - ✅
safe-commit.shals ergänzender Wrapper für AI-Agents
Potenzielle Probleme
1. Lint-Check läuft doppelt bei safe-commit.sh
safe-commit.sh führt npx lint-staged manuell aus und danach git commit (was wieder den Pre-Commit Hook triggert, der lint-staged nochmal ausführt). Das ist bewusst als Safety-Net, führt aber bei jedem Commit zu doppeltem Lint-Lauf.
Bewertung: Akzeptabel – ist explizit so designed, Performance-Impact minimal bei dieser Projektgröße.
2. lint:php in package.json vs. scripts/lint-php.sh
Das lint:php-Script in package.json ist ein komplexer Einzeiler, der die gleiche Aufgabe wie scripts/lint-php.sh hat, aber mit anderer Fehlerbehandlung (leitet alles nach /dev/null um, gibt weniger hilfreiche Fehlermeldungen). lint-staged nutzt das Shell-Script, was besser ist.
Bewertung: Nicht kritisch, aber könnte in Zukunft vereinheitlicht werden.
3. CI-Jobs installieren Pakete bei jedem Run
Kein Caching von apt-get/npm install. Bei dieser Projektgröße verschwindend gering.
Bewertung: Akzeptabel.
4. cp index.html entfernt
Die Zeile cp /deploy/haus-schleusingen.html /deploy/index.html wurde entfernt. Falls index.html nicht anderweitig bereitgestellt wird, könnte das zu einem 404 führen.
Bewertung: ⚠️ Prüfen! Wird index.html jetzt durch andere Mechanismen bereitgestellt (z.B. nginx rewrite)? Falls nicht, ist das ein Breaking Change.
Akzeptanzkriterien-Prüfung
Issue #53 – PR-Gate / Branch Protection
| Kriterium | Status | Anmerkung |
|---|---|---|
Branch-Schutz für main aktiviert: erfordert erfolgreiche Status-Checks vor Merge |
❌ Nicht im Code | Branch Protection ist eine Gitea-Repo-Einstellung, nicht im PR-Code konfigurierbar. Muss manuell im Repo-Admin gesetzt werden. |
Status-Checks lint-php, lint-css, lint-html als erforderlich konfiguriert |
❌ Nicht im Code | Gleicher Grund – Gitea-Setting. Die Jobs existieren aber korrekt im Workflow. |
| PR zeigt Test-Ergebnisse als Checks an | ✅ Erfüllt | Lint-Jobs laufen bei Push auf feature/**, werden im PR sichtbar |
| Deploy auf Testumgebung erfolgt nur bei grünen Tests | ✅ Erfüllt | needs: [lint-php, lint-css, lint-html] im deploy-test.yml |
| Dokumentation der Einstellungen im Repo | ⚠️ Teilweise | AGENTS.md aktualisiert, aber keine explizite Doku der Branch Protection Settings |
Issue #54 – Pre-Commit Hook als Gate
| Kriterium | Status | Anmerkung |
|---|---|---|
lint-staged enthält PHP Syntax-Check |
✅ Erfüllt | "*.{php}": ["scripts/lint-php.sh"] in package.json |
| Pre-Commit Hook prüft: PHP, HTML, CSS, JS, Prettier | ✅ Erfüllt | lint-staged + .husky/pre-commit covers all |
| Hook läuft auch bei AI-Agent / non-interactive | ✅ Erfüllt | safe-commit.sh + core.hooksPath-Setting |
| Klare Fehlermeldung bei Linter-Fehler | ✅ Erfüllt | lint-php.sh und safe-commit.sh haben klare Messages |
| README dokumentiert Hook-Aktivierung | ❌ Nicht erfüllt | AGENTS.md aktualisiert, aber README enthält keine Doku. Kriterium fordert explizit README. |
| AI-Agent-Anweisung in AGENTS.md | ✅ Erfüllt | Ausführliche Dokumentation vorhanden |
Fazit
Gesamtbewertung: CHANGES_REQUESTED (minor)
Was gut ist:
- Saubere Integration der Lint-Gates in den CI-Workflow
- Korrekte
needs-Abhängigkeit verhindert Deploy bei fehlgeschlagenen Lints - Gute Scripts mit fehlerresistentem Error-Handling
- Durchdachte AI-Agent-Unterstützung (
safe-commit.sh)
Was noch fehlt:
-
🔴
index.html-Entfernung prüfen: Das Entfernen voncp haus-schleusingen.html index.htmlim Deploy könnte die Seite kaputt machen. Bitte prüfen obindex.htmlanderweitig bereitgestellt wird. -
🟡 Issue #54 Kriterium "README dokumentiert": README.md enthält keine Anleitung zur Hook-Aktivierung. Entweder README ergänzen oder Akzeptanzkriterium anpassen.
-
🟡 Issue #53 Branch Protection: Kann nicht im Code gelöst werden – muss manuell in Gitea konfiguriert werden. Sollte nach PR-Merge manuell durchgeführt werden. Die CI-seitigen Voraussetzungen (Lint-Jobs) sind aber korrekt implementiert.