PR-Review gegen das Ticket durchführen
Das letzte Tor, bevor ein Pull-Request bereit ist. Es beantwortet eine Frage – liefert dieser Diff das Ticket, innerhalb der Architektur? – und beantwortet sie anhand zweier Quellen, die über dem Geschmack des Reviewers stehen: den Akzeptanzkriterien und der Referenzarchitektur.
Dieser Review liest und berichtet. Er editiert nicht: Ein Review, der das Gefundene behebt, vernichtet die Beweise dafür, was falsch lief, und niemand lernt das Muster.
Letzte von drei Phasen, nach TICKET-ANALYSIS.md und
IMPLEMENTATION-PHASE.md. WORKFLOW.md zeigt,
wie sie zusammenspielen und wie man sie installiert.
Inhalt
- Feststellen, was tatsächlich reviewed wird
- Akzeptanzkriterien zuerst
- Der Architektur-Durchlauf
- Der Test-Durchlauf
- Schweregrad und das Urteil
- Fehlermodi
- Checkliste
1. Den Diff ermitteln
Diffen Sie den Branch gegen den tatsächlichen Basis-Branch des Repositorys, nicht gegen eine Annahme, welcher das ist. Reviewen Sie jede geänderte Datei; eine Datei, die übersprungen wird, weil sie langweilig aussieht, ist die Stelle, an der die Konfigurationsänderung lauert.
Notieren Sie, was der Diff berührt, das das Ticket nicht erwähnt hat. Diese Liste ist eine Eingabe für §2 und §5, noch kein Befund.
2. Akzeptanzkriterien zuerst
Nehmen Sie die Akzeptanzkriterien einzeln und nennen Sie für jedes den Code, der es erfüllt, und den Test, der es beweist. Beides, oder es ist nicht erfüllt.
- Ein Kriterium mit Code und keinem Test ist unbewiesen.
- Ein Kriterium mit einem Test und keinem Code ist ein Test, der das Framework behauptet.
- Ein Kriterium, das Sie überhaupt nicht lokalisieren können, ist der Befund, der am meisten zählt, und er übertrifft jeden Stilkommentar im Review.
Dann die andere Richtung: Änderungen ohne dahinterliegendes Kriterium. Einige sind legitim – eine
Korrektur innerhalb des ohnehin geänderten Codes (IMPLEMENTATION-PHASE.md §2). Der Rest ist
Scope Creep, und es ist günstiger, ihn hier zu benennen, als nach dem Release zu erklären.
3. Der Architektur-Durchlauf
Gehen Sie die Compliance-Checkliste für die Schichten durch, die der Diff berührt. Die Befunde, die am meisten wert sind, sind die, die ein grüner Build nicht zeigen kann:
- P2 – etwas Domain-förmiges, das zum Shared Kernel hinzugefügt wurde: eine Entität, ein DTO, ein Enum, ein Seed-Datenset, ein Preiskonstante, eine benutzersichtbare Zeichenkette.
- P3 – ein kontextübergreifender Lese- oder Schreibzugriff; ein zweiter Dienst, der auf die Datenbank des besitzenden Dienstes zugreift.
- P4 – Schema geändert ohne Migration oder eine Migration, die nicht provider-portabel ist.
- P5 – ein Secret, Schlüssel oder Verbindungsstring im Quellcode, in der Konfiguration oder einem Kommentar; Konfiguration, die von woanders als der Umgebung gelesen wird.
- P8 – eine neue Abhängigkeit, die beim Start fehlschlägt, anstatt zu degradieren, oder eine, die im Health-Endpoint und im Startbanner fehlt.
- P9 / P10 – Verdrahtung, die in
Program.csinline ist; ein Erweiterungspunkt, der als Basisklasse statt als Schnittstelle, die in DI registriert ist, hinzugefügt wurde. - P11 – eine externe Form, die die Domain unübersetzt erreicht.
- P15 – ein neuer Pfad ohne Traces, Metriken oder Logs.
SERVICE-API-PATTERNS.md– ein Endpunkt außerhalb der Vertrauensgruppen, eine unbegrenzte Liste, ein uneinheitlicher Fehlerkörper, ein ausgehender Aufruf ohne explizites Timeout, ein wiederholter indeterminierter Schreibvorgang.- Sicherheit – alles, was neu öffentlich exponiert wird, erhält den
SECURITY-REVIEW.md-Durchlauf, bevor es ausgeliefert wird, nicht danach.
Eine Abweichung, die der Pull-Request als Entscheidung mit einem Grund dokumentiert, ist kein Befund. Die gleiche Abweichung, stillschweigend, ist einer – diese Unterscheidung ist P14 bei der Arbeit.
4. Der Test-Durchlauf
Lesen Sie die Testkörper. Testanzahl, Dateianzahl und Ordnerstruktur beweisen nichts; ein Audit am 2026-08-14 einer Suite mit 447 Tests ergab, dass etwa 45 % davon Platzhalter waren oder so abgesichert, dass sie nie etwas behaupteten, und nichts davon war von außen sichtbar (P13).
- Ist jeder neue Test auf der Schicht, die die Logik hält, oder steht ein Browser für einen Validator?
- Behauptet er ein Ergebnis oder dass ein Button geklickt wurde?
- Gibt es einen Regressionstest für jeden bestehenden Pfad, den der Diff berührt, oder nur Tests für den neuen Branch?
- Erfüllt er die Pro-Test-Hürde – unabhängig,
data-testid, keine Sleeps, keine Produktionsanmeldedaten (TESTING-STRATEGY.md§6)? - Wird ein neuer Test-Einstiegspunkt von einem CI-Kontext ausgeführt (§9)?
5. Schweregrad und das Urteil
Bewerten Sie jeden Befund, und zwar nach Konsequenz, nicht danach, wie einfach er zu beschreiben ist:
| Stufe | Was es bedeutet |
|---|---|
| Blockierend | Ein Akzeptanzkriterium ist nicht erfüllt oder nicht belegt; ein Prinzip ist ohne dokumentierte Entscheidung verletzt; ein Secret wurde offengelegt; ein Test wurde geschwächt oder gelöscht, um grün zu werden |
| Sollte behoben werden | Richtig, aber nicht dem Muster entsprechend: falsche Testschicht, fehlende Regressionsabdeckung, eine nicht dokumentierte Annahme, Dokumentation, die die Änderung unwahr gemacht hat |
| Überlegenswert | Tatsächlich optional. Halten Sie diesen Abschnitt kurz – ein langer vergräbt die ersten beiden |
Schließen Sie mit dem Urteil, das das Tor des Diagramms braucht, klar ausgedrückt: Liefert dieser Pull-Request das Ticket, und ist er innerhalb der Architektur? Eine von drei Antworten – validiert; validiert mit den aufgeführten nicht-blockierenden Befunden; oder nicht validiert, mit den blockierenden Befunden. „Sieht gut aus, ein paar Kommentare“ ist kein Urteil.
6. Fehlermodi
| Symptom | Ursache |
|---|---|
| Review genehmigt einen Diff, der eine Anforderung verfehlt | Der Durchlauf begann beim Code statt bei den Akzeptanzkriterien |
| Review ist eine Liste von Stilmeinungen | Die Architektur- und Testdurchläufe wurden übersprungen, weil der Build grün war – genau das, worauf sie prüfen |
| Eine grüne Suite wird als Beweis behandelt | Testkörper wurden nie gelesen; Zählung und Struktur wurden stattdessen vertraut |
| Befunde werden alle mit dem gleichen Gewicht gemeldet | Keine Schweregradeinstufung, sodass der blockierende Befund unter einem Namensnörgler liegt |
| Dieselbe Verletzung tritt über mehrere Pull-Requests hinweg auf | Befunde wurden vom Reviewer behoben anstatt gemeldet, sodass das Muster nie gelernt wurde |
| Eine Abweichung wird gemeldet, die der Autor bereits begründet hatte | Die dokumentierten Entscheidungen des Pull-Requests wurden vor dem Review nicht gelesen |
7. Checkliste
- Gegen den tatsächlichen Basis-Branch des Repositorys gedifft; jede geänderte Datei gelesen
- Jedes Akzeptanzkriterium auf den Code, der es erfüllt, und den Test, der es beweist, abgebildet
- Änderungen ohne dahinterliegendes Kriterium identifiziert und klassifiziert
- Compliance-Checkliste für jede vom Diff berührte Schicht durchgegangen
- Dokumentierte Entscheidungen zuerst gelesen, sodass gerechtfertigte Abweichungen nicht als Befunde gemeldet werden
- Alles neu Öffentliche mit dem Sicherheits-Review-Durchlauf geprüft
- Testkörper gelesen, nicht gezählt; Schicht, Ergebniseigenschaften, Regressionsabdeckung und die Pro-Test-Hürde geprüft
- Befunde als blockierend / sollte behoben werden / überlegenswert eingestuft
- Ein explizites Urteil gegen das Ticket abgegeben
- Während des Reviews nichts editiert
Generiert aus docs/delivery/PR-REVIEW.md von scripts/build-marketplace.mjs. Diese Datei nicht bearbeiten: Ändern Sie das Quelldokument oder seinen Eintrag in catalog/marketplace.catalog.json und führen Sie den Generator erneut aus.