Code-Review-Kultur: Wie du Feedback gibst und empfängst, das Code und Beziehungen verbessert

Code Review ist die eine Tätigkeit, bei der das Wissen, die Standards und die Gewohnheiten eines Teams tatsächlich von einer Person zur anderen übertragen werden. Es ist auch der Ort, an dem viele Teams leise Leute verlieren. Eine schlechte Code-Review-Kultur zeigt sich nicht in Metriken. Sie zeigt sich bei Entwicklern, die aufhören, PRs vor dem Mittagessen zu öffnen, weil sie sich nicht mit den Kommentaren herumschlagen wollen, und bei Reviewern, die alles genehmigen, weil Diskutieren anstrengend ist.

Wir arbeiten viel innerhalb von Kundenteams, wodurch wir sehen, wie viele verschiedene Teams Code reviewen. Die guten teilen ein paar Gewohnheiten. Dieser Beitrag handelt von diesen Gewohnheiten: wie man einen Review-Kommentar schreibt, damit er so gelesen wird, wie er gemeint war, wie man einen empfängt, ohne in Abwehrhaltung zu gehen, und welche strukturellen Regeln verhindern, dass das Ganze zu einem Flaschenhals wird.

Wofür ein Review-Kommentar da ist

Ein Review-Kommentar kann eine von vier Sachen tun. Er kann ein Merge blockieren, weil etwas falsch ist. Er kann eine Verbesserung vorschlagen, die der Autor ignorieren darf. Er kann eine Frage stellen, weil der Reviewer etwas nicht versteht. Oder er kann eine Kleinigkeit zu Stil oder Namensgebung anmerken, die kaum zählt.

Das Problem ist, dass in den meisten Teams alle vier gleich aussehen. "Sollte das nicht ein Repository-Call sein?" könnte "das wird in Produktion kaputtgehen" bedeuten oder "ich hätte es anders gemacht, aber egal". Der Autor muss raten, und Leute raten in die Richtung, es als Kritik zu nehmen.

Deshalb ist die erste Regel, die wir anwenden, den Kommentar zu labeln. Wir stellen jedem Review-Kommentar eines von wenigen Wörtern voran:

• blocking: bedeutet, der PR sollte nicht gemergt werden, bis das behoben ist. Der Reviewer schuldet eine Erklärung, warum.
• suggestion: bedeutet "ich würde X in Erwägung ziehen". Der Autor entscheidet.
• question: bedeutet, der Reviewer will etwas verstehen, nicht ändern. Oft ist die Antwort ein fehlender Kommentar im Code.
• nit: bedeutet Namensgebung, Formatierung, Reihenfolge. Beheb es, wenn du Lust hast.

Das ist keine neue Idee. Conventional Comments und ähnliche Schemata gibt es seit Jahren. Was zählt, ist, dass sich das Team auf die Wörter einigt und sie jedes Mal benutzt. Sobald die Labels existieren, liest sich ein PR mit zwölf Kommentaren, von denen elf nit: sind und einer question:, ganz anders als zwölf unlabelte Kommentare, obwohl der Text derselbe ist.

Feedback geben, das ankommt

Das Label setzt die Erwartung. Die Formulierung entscheidet, ob der Autor dich hört.

Erkläre den Grund, nicht nur die Änderung. "Benutz hier ein DTO" ist eine Anweisung. "Die Entity direkt in die Response zu geben bedeutet, dass jede neue Spalte auf der Tabelle in die API durchsickert. Ein DTO hält diese Grenze explizit" ist ein Grund, und Gründe sind der Teil, der Wissen überträgt. Zwei Monate später trifft der Autor die richtige Entscheidung von selbst, weil er es verstanden hat, nicht weil man es ihm gesagt hat.

Mach es zum Vorschlag, wenn es tatsächlich einer ist. Es gibt einen Unterschied zwischen "das ist falsch" und "ich bevorzuge das". Reviewer, die Präferenzen als Anforderungen formulieren, trainieren Autoren darauf, alle ihre Kommentare zu ignorieren, auch die wichtigen. Wenn du den PR sowieso genehmigen würdest, sag das: "Genehmigt. Ein Vorschlag unten, nimm ihn oder lass ihn."

Frag, bevor du behauptest. Wenn etwas falsch aussieht und du dir nicht sicher bist, ist "Was passiert, wenn $order hier null ist?" besser als "Das wird bei null abstürzen". Die Hälfte der Zeit gibt es einen Guard, den du drei Dateien weiter oben übersehen hast. Die andere Hälfte findet der Autor den Bug selbst beim Antworten, was ein besseres Ergebnis ist, als gesagt zu bekommen.

Review den Code, nicht die Person. "Du vergisst immer, den Fehlerfall zu behandeln" ist ein Personalgespräch, kein Review-Kommentar. "Der Fehlerfall in Zeile 41 wird nicht behandelt" ist ein Review-Kommentar. Halte die beiden getrennt.

Überlass Kleinigkeiten dem Tooling. Wenn das Team in Reviews über die Platzierung von geschweiften Klammern streitet, ist die Lösung ein Formatter in CI, keine weiteren Reviews. Jedes nit:, das ein Linter hätte finden können, ist Aufmerksamkeit, die den Kommentaren fehlt, die zählen. Das ist auch der günstigste Gewinn, wenn wir mit einem neuen Kundenteam starten: die Stildebatten in der ersten Woche aus der menschlichen Schleife herausnehmen.

Feedback annehmen, ohne zu streiten

Die andere Hälfte der Code-Review-Kultur ist schwerer, Regeln dafür zu schreiben, weil es meist darum geht, nicht in Abwehrhaltung zu gehen, und niemand entscheidet sich bewusst, defensiv zu sein.

Ein paar Dinge helfen. Das Erste ist, sich daran zu erinnern, dass der Reviewer weniger Kontext hat als du. Du hast zwei Tage in diesem Code verbracht. Er hat zwanzig Minuten verbracht. Wenn ein Kommentar offensichtlich falsch wirkt, ist die wahrscheinlichste Erklärung, dass dem Reviewer etwas fehlt, das du weißt, und die Lösung ist, es zu erklären, nicht zu argumentieren. "Gute Frage. Das ist null-sicher, weil die Abfrage oben nach deleted_at IS NULL filtert, ich füge einen Kommentar hinzu" löst den Thread auf und verbessert den Code.

Das Zweite ist, den Unterschied zwischen Kontext erklären und eine Entscheidung verteidigen zu bemerken. Kontext erklären ist "ich habe es so gemacht, weil der Zahlungsanbieter bei 5xx wiederholt und wir Idempotenz brauchen". Verteidigen ist "das ist okay, es funktioniert". Das Erste lädt den Reviewer ein, es sich noch einmal anzusehen. Das Zweite schließt einfach die Tür. Wenn du dich dabei ertappst, "es funktioniert" zu schreiben, halt inne und frag dich, ob du wirklich sicher bist, dass der Reviewer falsch liegt, oder nur müde bist.

Das Dritte ist, um Klärung zu bitten, bevor du zurückdrängst. Erstaunlich viele Review-Streits sind zwei Leute, die sich in unterschiedlichen Worten einig sind. "Kannst du genauer sagen, was du hier anders machen würdest?" kostet zehn Sekunden und verhindert einen Fünfzehn-Kommentar-Thread.

Und wenn der Reviewer recht hat, sag das und mach weiter. "Ja, behoben" ist eine vollständige Antwort. Niemand braucht die Geschichte, warum der Bug da war.

Feedback zu Code ist kein Feedback zu dir. Dieser Satz ist leicht geschrieben und schwer zu fühlen, besonders für weniger erfahrene Entwickler, und besonders wenn der Reviewer nachlässig mit der Formulierung ist. Deshalb zählt die Geberseite so sehr: Reviewer, die labeln und erklären, machen es Autoren viel leichter, ruhig zu bleiben.

Die strukturellen Regeln, die Code-Review-Kultur funktionieren lassen

Gute Absichten überleben keinen 2.000-Zeilen-PR. Die meisten Review-Probleme, die wir sehen, sind strukturell, und die Lösungen sind Richtlinien statt Einstellungen.

Die PR-Größe begrenzen

Die Review-Qualität sinkt stark, wenn PRs größer werden. Die oft zitierte SmartBear-Studie zu Cisco-Reviews setzte die Grenze bei etwa 400 Zeilen Code, danach hören Reviewer auf, Defekte zu finden, und fangen an zu überfliegen. Unsere eigene Erfahrung deckt sich damit. Jenseits von ein paar hundert Zeilen genehmigen Reviewer nach Gefühl.

Wir verwenden ein weiches Limit von 400 geänderten Zeilen und die harte Erwartung, dass alles Größere aufgeteilt wird oder mit einer schriftlichen Begründung kommt. Generierter Code, Lockfiles und Migrationen zählen nicht mit. Wenn sich ein Feature nicht in überprüfbare Teile aufteilen lässt, ist das meist ein Zeichen, dass das Design Arbeit braucht, und ein Design-Gespräch, bevor der Code geschrieben wird, ist günstiger als ein Review danach.

Eine Reaktionszeit-Regel setzen

Ein PR, der drei Tage liegen bleibt, blockiert den Autor, wird gegenüber dem Basis-Branch veraltet und wird von jemandem reviewt, der den Faden verloren hat. Wir verlangen eine erste Reaktion innerhalb eines Arbeitstags. Das muss kein vollständiges Review sein; "Ich schaue mir das morgen früh an" zählt. Der Punkt ist, dass der Autor weiß, was zu erwarten ist, und danach planen kann.

Für Teams, die damit kämpfen, ist die Lösung meist, Review zu einem geplanten Teil des Tages zu machen statt zu etwas, das zwischen Aufgaben gequetscht wird. Ein Team, das jeden Morgen mit dreißig Minuten offenen PRs beginnt, hat sehr wenige veraltete.

Für asynchron schreiben

Verteilte Teams können sich nicht darauf verlassen, an den Schreibtisch von jemandem zu gehen. Das verändert, wie ein PR geschrieben werden sollte. Die Beschreibung muss den Kontext tragen, den ein Gespräch sonst liefern würde: was die Änderung tut, warum, wobei du dir unsicher bist, und worauf der Reviewer sich konzentrieren soll. "Bitte prüf die Transaktionsgrenze in OrderService::confirm, ich bin mir nicht sicher, ob die Retry-Logik stimmt" ist der nützlichste Satz in jeder PR-Beschreibung, und fast niemand schreibt ihn.

Self-Review, bevor du ein Review anfragst. Lies deinen eigenen Diff von oben nach unten, als wärst du der Reviewer. Du wirst das übrig gebliebene dump(), den auskommentierten Block und den Tippfehler in der Migration finden, und der Reviewer muss es nicht mehr.

Die Reviews genauso retrospektiv betrachten wie den Sprint

Alle paar Monate lohnt sich ein Blick auf den Review-Prozess selbst. Welche PRs lagen am längsten und warum? Welche Threads gingen über zehn Kommentare hinaus? Waren die blocking:-Kommentare tatsächlich blockierend, oder ist das Label abgedriftet? Bekommt jemand konsequent härteres Feedback als andere? Diesen letzten Punkt lohnt es sich, absichtlich zu prüfen, weil er selten von allein auffällt.

Wie wir das in Kundenteams einbringen

Wenn Wolf-Tech zu einem bestehenden Team stößt, kommen wir nicht mit einer Review-Richtlinie an und bitten alle, sie zu übernehmen. Das funktioniert nie. Was wir stattdessen tun, ist, auf die oben beschriebene Weise zu reviewen, mit Präfixen, Gründen und kleinen PRs unsererseits, und das Team sehen zu lassen, wie das aussieht. Die meisten Teams übernehmen die Labels innerhalb weniger Wochen, weil sie die Kommentare leichter lesbar machen.

Die strukturellen Regeln brauchen länger, und wir schlagen sie meist nach dem ersten Retro vor, wenn die Schmerzpunkte schon sichtbar sind. Ein PR-Größenlimit ist viel leichter zu vereinbaren, direkt nachdem ein 3.000-Zeilen-PR zwei Wochen lang offen war.

Reviews sind auch der Hauptkanal, über den wir tiefere Probleme finden. Eine Codebasis, in der jeder PR zehn blocking:-Kommentare braucht, hat ein Problem vor dem Review. Dort kommt meist ein Code-Audit oder ein Modernisierungsplan ins Spiel, denn man kann sich nicht aus einer Architektur herausreviewen, die gegen die Entwickler arbeitet.

Eine kurze Checkliste

Für Reviewer: labele jeden Kommentar, erkläre den Grund hinter allem, was blockiert, frag, wenn du unsicher bist, und halte Stil aus der menschlichen Schleife raus.

Für Autoren: halte PRs unter 400 Zeilen, schreib eine Beschreibung, die sagt, worauf man achten soll, mach zuerst ein Self-Review, und behandle die Verwirrung eines Reviewers als Dokumentationslücke statt als Beleidigung.

Für das Team: eine erste Reaktion innerhalb eines Arbeitstags, ein Formatter in CI, und alle paar Monate ein Review-Retro.

Nichts davon ist kompliziert. Der schwere Teil ist, es konsequent zu tun, wenn der Sprint im Rückstand ist und alle müde sind, was genau dann ist, wenn ein unachtsamer Review-Kommentar den größten Schaden anrichtet.

Wenn sich die Reviews deines Teams entweder in Gummistempel oder Grabenkämpfe verwandelt haben, schauen wir uns gerne ein paar aktuelle PRs an und sagen dir, was wir sehen. Schreib an hello@wolf-tech.io oder lies mehr darüber, wie wir mit Teams arbeiten, auf wolf-tech.io.