Code-Review-Checkliste
Code-Review-Checkliste für Pull Requests: Absicht, Korrektheit, Tests, Sicherheit, Datenänderungen und Lesbarkeit. Für riskante PRs, mit Nachweis des Reviews.
- Typischer Rhythmus pro Pull Request
- Die Checkliste 6 Abschnitte, 19 Punkte
- Variablen
{{repository}}{{pr_number}} - Aus der CI starten
Diese Checkliste ist für Pull Requests gedacht, die mehr als einen kurzen Blick verdienen: Änderungen an Authentifizierung, Zahlungen, Datenlöschung, Berechtigungen, öffentlichen APIs oder Datenbankschemas. Für kleine Routineänderungen reicht das normale Review. Die Checkliste lenkt den Blick auf das, was leicht übersehen wird, wenn man den Diff nur von oben nach unten liest.
Wer verantwortlich ist
Die Person, die reviewt, ist für den Durchlauf verantwortlich. Die Autorin oder der Autor bereitet den Pull Request so vor, dass er reviewbar ist: klare Beschreibung, verlinktes Ticket und Tests. Wird eine zweite Person mit Spezialwissen gebraucht, etwa für Sicherheit oder die Datenbank, ihr die entsprechenden Punkte zuweisen. In Teams, die für ein Audit Peer Reviews nachweisen müssen, ist der abgeschlossene Durchlauf dieser Nachweis.
Typische Fehler
- Stil statt Verhalten prüfen. Formatierung und Benennung erledigen Linter und Formatter. Die Zeit im Review für Logik, Grenzfälle und Fehlerverhalten nutzen.
- Zu große Pull Requests. Einen riesigen Diff reviewt niemand gut. Um eine Aufteilung bitten, statt auf Vertrauen freizugeben.
- Nur das Geänderte lesen. Fehler stecken oft im unveränderten Code, der jetzt anders aufgerufen wird. Die Aufrufer ansehen.
- Freigeben, ohne etwas auszuprobieren. Bei Änderungen an Oberfläche oder Verhalten den Branch auschecken oder die Preview-Umgebung öffnen und testen.
- Unklare Kommentare. Kennzeichnen, welche Kommentare den Merge blockieren und welche Vorschläge sind, damit klar ist, was sich ändern muss.
Die Checkliste: Code-Review {{repository}} PR {{pr_number}}
Kontext
-
PR
{{pr_number}}hat eine Beschreibung, die erklärt, was sich ändert und warum, mit verlinktem Ticket -
Die Änderung ist klein genug für ein gründliches Review, oder um eine Aufteilung wurde gebeten
-
CI-Checks, Linter und Tests laufen auf dem neuesten Commit grün
Korrektheit
-
Der Code tut, was das Ticket verlangt, einschließlich der dort genannten Grenzfälle
-
Fehlerbehandlung deckt Ausfälle externer Aufrufe, leere Eingaben und Timeouts ab
-
Aufrufer geänderter Funktionen verhalten sich weiterhin korrekt
-
Nebenläufigkeit, Wiederholungen und Idempotenz berücksichtigt, wo Jobs oder Requests doppelt laufen können optional
Tests
-
Neues Verhalten ist durch Tests abgedeckt, die ohne die Änderung fehlschlagen würden
-
Tests prüfen Ergebnisse, nicht nur, dass der Code ohne Fehler läuft
-
Änderung im Branch-Build oder in der Preview-Umgebung von Hand ausprobiert optional
Sicherheit und Daten
-
Eingaben von Nutzern oder anderen Systemen werden validiert, Ausgaben bei Bedarf kodiert
-
Autorisierung für jeden neuen Endpunkt und jede neue Aktion geprüft
Sicherstellen, dass niemand durch Ändern einer ID an Daten anderer Nutzer oder Organisationen kommt.
-
Keine Secrets, Tokens oder personenbezogenen Daten committet oder in Logs geschrieben
-
Neue Abhängigkeiten sind nötig, gepflegt und haben eine passende Lizenz optional
-
Datenbankmigrationen sind abwärtskompatibel und sicher auf großen Tabellen optional
Wartbarkeit
-
Namen und Struktur machen den Code auch ohne PR-Beschreibung verständlich
-
Doku, Kommentare oder Changelog bei geändertem Verhalten aktualisiert
-
Review-Kommentare als blockierend oder als Vorschlag gekennzeichnet
Entscheidung
-
Ergebnis des Reviews festgehalten: freigegeben, Änderungen angefordert oder zweites Review nötig
Checkliste als Markdown
Diese Checkliste aus der Pipeline starten
Wenn ein Pull Request das Label high-risk bekommt, POST /api/v1/runs mit der Vorlage "Code-Review {{repository}} PR {{pr_number}}" und der PR-Nummer als pr_number aufrufen.
curl -X POST https://runbook.metastic.ch/api/v1/runs \
-H "Authorization: Bearer $RUNBOOK_TOKEN" \
-H "Content-Type: application/json" \
-d '{"template": "Code-Review {{repository}} PR {{pr_number}}", "variables": {"repository": "…", "pr_number": "…"}}'Häufige Fragen
Was sollte man beim Code-Review zuerst prüfen?
Mit der Absicht beginnen: Beschreibung und Ticket lesen und entscheiden, ob der Ansatz sinnvoll ist, bevor es um Details geht. Danach Korrektheit und Tests prüfen, dann Sicherheit und Umgang mit Daten. Stilfragen kommen zuletzt, und die meisten davon sollten automatisiert sein.
Wie groß sollte ein Pull Request für ein gutes Review sein?
Klein genug, dass man ihn in einer Sitzung versteht. Eine feste Zeilenzahl gibt es nicht, aber wer die ganze Änderung nicht im Kopf behalten kann, hat einen zu großen Pull Request vor sich. Ihn in Refactoring und Verhaltensänderung oder in Backend- und Frontend-Teil aufteilen.
Braucht jeder Pull Request eine Checkliste?
Meist nicht. Eine Checkliste für jede Kleinigkeit wird schnell zum reinen Abhaken. Sie für riskante Änderungen nutzen, die sich im Git-Host mit einem Label markieren lassen, und den Durchlauf aus der CI starten, sobald das Label gesetzt wird.
Aktualisiert