Migrate all repos into monorepo context folders
Bahn: aisupport, Analyse-O2C-C2S, awesome-bahn-mcp-servers, beam-mcp,
Confluence_Bot, db-planet-mcp-server, O2C-Harness, project-audit,
Projekt-KIQ-HP, teamlandkarte-mcp
Dhive: Jury-Voting
Privat: CV, NoteGraph (NOTE: NoteGraph needs complete redo after consolidation)
Shared: AI-Orchestrator, OrgMyLife, power_skills_and_more
Shared/references: symphony (read-only)
Bahn repos remain available as independent remotes - this monorepo
pulls them in via subtree, the originals are untouched.
This commit is contained in:
@@ -0,0 +1,105 @@
|
||||
# Code-Review (Self-Review mit GitLab Discussions)
|
||||
|
||||
**WICHTIG:** Code-Review erst durchführen wenn die MR-Pipeline GRÜN ist.
|
||||
Reihenfolge: Push → MR erstellen → Pipeline abwarten → Pipeline grün → Code-Review → Findings fixen → Merge.
|
||||
Nicht reviewen solange Pipeline noch läuft oder failed ist – erst Pipeline fixen!
|
||||
|
||||
## Ablauf nach MR-Erstellung (ERST wenn MR-Pipeline grün!)
|
||||
|
||||
1. **Code lokal reviewen** (nicht nur Diff, sondern gesamten Code)
|
||||
2. **Findings als GitLab Discussions anlegen**
|
||||
3. **Findings selbst fixen**
|
||||
4. **Threads resolven**
|
||||
5. **Erst mergen wenn alle Threads resolved**
|
||||
|
||||
## Review-Kriterien
|
||||
|
||||
### Clean Code
|
||||
- Single Responsibility Principle
|
||||
- Sprechende Namen (Variablen, Methoden, Klassen)
|
||||
- Kleine Methoden (max 20 Zeilen)
|
||||
- Kein toter Code, keine auskommentierten Blöcke
|
||||
- DRY (Don't Repeat Yourself)
|
||||
|
||||
### Separation of Concerns
|
||||
- Controller nur für HTTP-Handling (kein Business-Logic)
|
||||
- Service-Layer für Business-Logik
|
||||
- Repository nur für Datenzugriff
|
||||
- DTOs für API-Kommunikation (nicht Entity direkt exponieren)
|
||||
|
||||
### Best Practices
|
||||
- Input-Validierung (@Valid, Bean Validation)
|
||||
- Fehlerbehandlung (Exception Handler, sinnvolle HTTP Status Codes)
|
||||
- Logging an kritischen Stellen
|
||||
- Keine Secrets/Credentials im Code
|
||||
- Immutable wo möglich
|
||||
|
||||
### Tests
|
||||
- Alle öffentlichen Methoden getestet
|
||||
- Edge Cases abgedeckt (null, leer, ungültig)
|
||||
- Given-When-Then Struktur
|
||||
- Mocks nur wo nötig (keine Over-Mocking)
|
||||
|
||||
### Security
|
||||
- Keine SQL Injection (Parameterized Queries / JPA)
|
||||
- Keine XSS (Output Encoding)
|
||||
- Keine Secrets im Code
|
||||
- Dependencies ohne bekannte CVEs
|
||||
|
||||
## GitLab Discussion anlegen
|
||||
|
||||
```bash
|
||||
# Discussion an einer bestimmten Datei/Zeile
|
||||
glab api -X POST "projects/:id/merge_requests/{mr_iid}/discussions" \
|
||||
-f "body=**Clean Code:** Diese Methode hat zu viele Verantwortlichkeiten. Bitte in separate Methoden aufteilen." \
|
||||
-f "position[base_sha]=$(git merge-base main HEAD)" \
|
||||
-f "position[start_sha]=$(git merge-base main HEAD)" \
|
||||
-f "position[head_sha]=$(git rev-parse HEAD)" \
|
||||
-f "position[position_type]=text" \
|
||||
-f "position[new_path]=src/main/java/com/example/controller/AddressController.java" \
|
||||
-f "position[new_line]=42"
|
||||
|
||||
# Allgemeine Discussion (nicht an Datei gebunden)
|
||||
glab api -X POST "projects/:id/merge_requests/{mr_iid}/discussions" \
|
||||
-f "body=**Architektur:** Service-Layer fehlt. Controller greift direkt auf Repository zu. Bitte AddressService einführen."
|
||||
```
|
||||
|
||||
## Findings fixen und Thread resolven
|
||||
|
||||
```bash
|
||||
# Fix implementieren
|
||||
# ... Code ändern ...
|
||||
git add -A && git commit -m "fix(review): Service-Layer einführen (#issue)"
|
||||
git push
|
||||
|
||||
# Thread resolven
|
||||
glab api -X PUT "projects/:id/merge_requests/{mr_iid}/discussions/{discussion_id}" \
|
||||
-f "resolved=true"
|
||||
```
|
||||
|
||||
## Merge-Bedingung
|
||||
|
||||
```bash
|
||||
# Prüfen ob alle Threads resolved sind
|
||||
UNRESOLVED=$(glab api "projects/:id/merge_requests/{mr_iid}/discussions" | python3 -c "
|
||||
import sys, json
|
||||
discussions = json.load(sys.stdin)
|
||||
unresolved = [d for d in discussions if any(n.get('resolvable') and not n.get('resolved') for n in d.get('notes',[]))]
|
||||
print(len(unresolved))
|
||||
")
|
||||
|
||||
if [ "$UNRESOLVED" = "0" ]; then
|
||||
echo "Alle Threads resolved - merge möglich"
|
||||
glab mr merge --when-pipeline-succeeds
|
||||
else
|
||||
echo "Noch $UNRESOLVED offene Threads - erst fixen"
|
||||
fi
|
||||
```
|
||||
|
||||
## Wichtig
|
||||
|
||||
- Review den GESAMTEN Code, nicht nur den Diff
|
||||
- Mindestens 3 Review-Punkte prüfen (auch wenn alles gut aussieht → "LGTM" als Discussion)
|
||||
- Findings mit Kategorie-Prefix: **Clean Code:**, **Security:**, **Architektur:**, **Test:**
|
||||
- Jedes Finding muss actionable sein (konkreter Verbesserungsvorschlag)
|
||||
- Erst mergen wenn 0 unresolved Threads
|
||||
Reference in New Issue
Block a user