Wartbarkeits-Review
Es gibt nur ein Kriterium: Wie viele Stellen muss die nächste Person (Mensch oder Agent) öffnen, um diesen Code zu ändern? Es werden keine Korrektheitsfehler behandelt. Das ist Aufgabe des Code- oder PR-Review.
0. Modus
| Modus | Ziel | Vergleichsbasis | Blockierende Probleme |
|---|---|---|---|
| Selbstcheck | mein Diff vor dem Commit | gesamtes Repo | Ich ändere es (ohne Bericht) |
| Änderungs-Review | PR-Diff | gesamtes Repo | Nur durch die Änderung verursachte Probleme |
| Überprüfung | gesamtes Modul oder Verzeichnis | gesamtes Repo | Alle. In der Reihenfolge der Auswirkungen |
Bei einem Änderungs-Review werden ursprünglich vorhandene Probleme nicht als blockierend gemeldet. Sie werden separat als „bestehende Altlast“ notiert.
1. Projektstandards lesen
- Suche nach
docs/review/architecture.md. Falls nicht vorhanden, verwende das vonCLAUDE.md/AGENTS.mdreferenzierte Architekturdokument. - Projektstandards haben Vorrang vor den allgemeinen Standards dieses Skills. Insbesondere werden folgende Punkte aus dem Standarddokument übernommen:
- Modulkarte und Abhängigkeitsrichtung
- Liste der wiederzuverwendenden Abstraktionen (HTTP-Client, Ergebnistypen, Test-Helfer usw.)
- Liste bekannter Altlasten. Muster in dieser Liste sind kein Vorbild, dem neuer Code folgen sollte.
- Wenn kein Standarddokument existiert, zeichne die Modulkarte selbstständig anhand der Verzeichnisstruktur und der
index-Exporte jedes Moduls und vermerke in der ersten Zeile des Berichts: "Kein Standarddokument".
2. Worauf achten
Von oben nach unten vorgehen. Je weiter oben, desto höher sind die späteren Kosten und desto häufiger übersehen Agenten dies.
2-1. Wurden bestehende Abstraktionen umgangen?
Ein Agent arbeitet nur an einem Issue und erstellt daher dasselbe erneut, ohne die von einem vorherigen Issue erstellten Module zu kennen. Dies ist der häufigste und teuerste Fehler.
Suche für jede neu erstellte Funktion, jeden Typ, jede Fehlerklasse, Konstante und jeden externen Aufruf nach Code im Repository, der dasselbe tut.
git grep -nE "class \w+Error extends" # Fehlerklassen
git grep -nE "https?://[a-z.]+\\.(com|io|dev)" # Externe API-Adressen und Header
git grep -nE "^(export )?type \w+ = \{" # Typen mit ähnlichem Namen (z. B. XxxEntry)
git grep -nF "<charakteristischer String oder Regex des neuen Codes>"
- Wenn es mehr als einen Wrapper gibt, der dasselbe externe System aufruft, müssen Aufrufer Fehler in mehrere Klassen aufteilen. Suche auch nach Spuren davon (
a instanceof X || b instanceof Y). - Wenn dieselbe Listenkonstante (Erweiterungen, Statuswerte) an zwei Stellen vorkommt, wird nur eine Seite geändert.
2-2. Abhängigkeitsrichtung
- Wurden die Layer-Regeln des Standarddokuments verletzt? (z. B. importiert ein Domänenmodul die UI oder eine externe API).
- Importiert ein untergeordnetes Modul ein übergeordnetes Modul, oder importieren zwei Module einander?
2-3. Einheiten
Nicht nach Größe teilen, sondern nach Änderungsgrund.
- Wenn eine Datei zwei oder mehr Gruppen von Funktionen enthält, die sich nicht gegenseitig aufrufen, ist dies ein Zeichen für mehr als einen Änderungsgrund (z. B. Formular-Mapping + Zugriff auf verschachtelte Schlüssel + Datumsformatierung).
- Wenn mehrere Bildschirmdateien dieselbe sequentielle Vorbereitung wiederholen (Authentifizierung → Konfiguration laden → Fehlerverzweigung), sollte dieser Prozess eine benannte Funktion sein.
2-4. Qualität der Abstraktion
- Fehlende API: Wenn zwei oder mehr Aufrufer dieselbe Vor- oder Nachverarbeitung wiederholen, fehlt dem Modul eine Funktion für diese Verarbeitung. Beispiel: Wenn alle Aufrufer nach
doc.items.find((s) => s.kind === "x")auchparseX(...)aufrufen, fehltreadX(doc). - Übermäßige Abstraktion: Schnittstellen, Factories, Optionen oder Generika, die nur an einer Stelle verwendet werden. Dies sind derzeit nicht benötigte Erweiterungspunkte.
- Typen an den Grenzen: Wenn der Aufrufer Fehler behandeln muss, sollte dies durch Ergebnistypen (diskriminierte Unions) sichtbar sein. Vergleicht der Aufrufer Nachrichtenstrings zur Verzweigung?
2-5. Muster
- Wurden die bereits im Repository verwendeten Muster (Ergebnistypen, Dependency Injection, Fehlerbehandlung, Modul-Exporte) befolgt?
- Neue Muster werden nur akzeptiert, wenn es einen Grund gibt, warum die bestehenden Muster nicht ausreichen. Fehlt dieser Grund im PR oder in Kommentaren, wird es beanstandet.
2-6. Kommentare und Namen
- Sind Kommentare am falschen Ziel angebracht? Ein häufiger Unfall: Änderungscommits fügen eine neue Funktion zwischen bestehenden Funktionen ein, sodass der JSDoc-Kommentar über einer anderen Funktion landet.
- Stimmt der Name mit der Funktion überein? Wenn der Name eine andere Funktionsweise erwarten lässt, wird dies beanstandet.
3. Bewertung
blocking (Nicht mergen)
- Code mit derselben Verantwortung existiert bereits, wurde aber neu erstellt – es sei denn, der PR oder ein Kommentar begründet, warum der bestehende nicht verwendet werden kann.
- Verstoß gegen die Layer-Regeln des Standarddokuments.
- Neue API zwingt zwei oder mehr Aufrufer, dieselbe Verarbeitung zu wiederholen.
- Ein Kommentar ist am falschen Objekt angebracht und führt den Leser in die Irre.
suggestion (Merge nicht blockieren, sondern als Folgetask zurückstellen)
- Dateitrennung, Namensverbesserung, ursprünglich vorhandene Dubletten.
Nicht erwähnen: Geschmacksfragen (funktionsorientiert vs. imperativ, Zeilenlänge), Punkte die Linter und Formatierer abdecken, Einführung neuer Bibliotheken außerhalb des Issue-Umfangs.
4. Verifikation
Jede Beanstandung muss durch Code belegt werden. Falsche Beanstandungen verschwenden die Zeit des implementierenden Agenten.
- Bei "Duplikat": Öffne beide Stellen und prüfe, ob sie wirklich dasselbe tun. Wenn sie ähnlich aussehen, aber unterschiedliche Spezifikationen einhalten, lasse sie aus.
- Bei "N Stellen": Zähle mit grep.
- Bei "Durch diese Änderung verursacht": Prüfe mit
git diff <base>...HEAD, ob die betreffenden Zeilen Teil des Diffs sind.
5. Bericht
Erste Zeile: Umfang (Modus, Zielpfad, Vorhandensein des Standarddokuments) und Anzahl (blocking N, suggestion N, bestehende Altlast N).
Pro Eintrag:
- Titel: Das Ergebnis beschreibt die Kosten. ("GitHub 401 wird vom Aufrufer in 3 Fehlerklassen geprüft" ○ / "Fehlerbehandlung verbessern" ×)
- Position: Alle relevanten Positionen als
Datei:Zeile. Bei Duplikaten beide Seiten. - Kosten: Wie viele Stellen müssen geändert werden, wenn als nächstes etwas geändert wird. ("Wenn der API-Versionsheader geändert wird, müssen 3 Dateien geändert werden")
- Richtung: Ein bis zwei Sätze, in welche Richtung zusammengeführt werden soll.
- Stufe: blocking / suggestion / bestehende Altlast.
Wenn es nichts zu beanstanden gibt, schreibe "Keine". Erfinde keine Beanstandungen, um den Bericht zu füllen.
Was nicht zu tun ist
- Aufteilen nur aufgrund von Zeilenanzahl oder Dateigröße verlangen.
- Richtungslose Beanstandungen ("Es wäre besser, dies zu abstrahieren").
- Code, der nur ähnlich aussieht, als Duplikat bezeichnen.
- Ursprünglich vorhandene Probleme als blockierend für diesen PR melden.