Dieser Artikel zeigt euch beispielhaft, wie ihr mit eurem Team Regeln für Code Reviews erarbeiten könnt und welche Punkte ihr dabei beachten solltet.
Code Reviews werden typischerweise innerhalb des verwendeten Source-Code-Management-Systems wie GitLab oder GitHub ausgeführt und dokumentiert. Meinungen oder auch Kritik an der Arbeit einer anderen Person werden dadurch rein textbasiert kommuniziert – das birgt ein gewisses Konfliktpotential. Der gestiegene Anteil von mobiler Arbeit verschärft dieses Konfliktpotential noch, da eine direkte persönliche Klärung z. B. im Büro oft gar nicht oder nur über Videokonferenzen möglich ist.
Gibt es bereits unterschwellige Konflikte im Team, kann eine unüberlegte Formulierung in einem Kommentar zu einem emotionalen Streit zwischen zwei Entwickler:innen führen. Deshalb ist es sinnvoll, das Team für das Thema Kommunikation in Code Reviews zu sensibilisieren und sich auf gemeinsame Richtlinien zu einigen. Wir zeigen euch hier beispielhaft den Ablauf eines Workshops, wie wir dazu vorgegangen sind:
Wozu machen wir Code Reviews?
Zuerst klären wir mit dem Team, was die Ziele von Code Reviews sind. Welche Themen sollen angesprochen werden, wozu ist Feedback erwünscht?
Dies dient als Grundlage für die folgenden Schritte, aber auch zur Abgrenzung: Wozu dienen unsere Code Reviews nicht? Wird der Code z. B. nochmal gegen die Spezifikation geprüft oder sogar ausgeführt?
Die verschiedenen Motivationen können helfen, später andere Entscheidungen zu treffen: Ob eine Codestelle eine Sicherheitslücke darstellt oder die Formatierung anders sein könnte, wird vermutlich zu anderen Reaktionen von Reviewer:innen führen.
Objektive und subjektive Kriterien
Anhand der Liste von verschiedenen Zielsetzungen für das Code Review kategorisieren wir gemeinsam, welche Prüfpunkte sich objektiv entscheiden lassen und welche immer subjektiv sind.
Objektive Kriterien sollten möglichst automatisiert in der CI/CD Pipeline geprüft werden. Da sie objektiv entscheidbar sind, wäre es Verschwendung, wenn ein Mensch Zeit darauf investiert. Typische Beispiele sind Codeabdeckungen von Unit Tests oder Einhaltung von Formatierungen. Statische Werkzeuge zur Codeanalyse wie SonarQube bieten umfangreiche Regelsets, die jedes Team an seine eigenen Regeln anpassen kann.
Je nach Zeitrahmen kann in dieser Phase mit dem Team über konkrete Messwerte/Regeln gesprochen werden, die z. B. Teil einer Definition of Done werden.
Die verbliebene Liste an subjektiven Kriterien dient dazu, das Team zu sensibilisieren, dass es hier kein einfaches richtig oder falsch gibt, sondern möglicherweise Meinungen aufeinander stoßen. Um möglichst effizient mit unterschiedlichen Meinungen umzugehen, sprechen wir mit dem Team die Standardreaktionen für subjektive Situationen ab.
Verhalten in subjektiven Situationen
Es gibt grundsätzlich zwei mögliche Reaktionen von Reviewer:innen bei einem Pull/Merge Request mit subjektiven Meinungsunterschieden:
- man hinterlässt Feedback, aber akzeptiert die Änderung trotzdem
- man hinterlässt Feedback und akzeptiert die Änderung nicht
Eine Änderung ohne Feedback zu akzeptieren sollte der Normalfall sein. Eine Änderung ohne Feedback nicht zu akzeptieren ist nicht sinnvoll, da dem/der Autor:in nicht klar wird, wieso eine Änderung abgelehnt wird.
Das heißt, eine Reviewer:in muss entscheiden, ob das eigene Feedback und die Motivation für dieses Feedback so wichtig ist, dass der Pull/Merge Request blockiert wird. In allen anderen Fällen ist das Feedback optional, es kann von der Autor:in umgesetzt werden oder nicht.
Mit dem Team diskutieren wir, für welche Motivationen welche Reaktion der Reviewer:innen Standard sein sollte. Das Ergebnis könnte z. B. so aussehen:
- Motivation: Bug auf Produktion vermeiden oder Sicherheitsbedenken => Änderung nicht akzeptieren.
- Motivation: bei Legacy-Code ausreichend Tests sicherstellen => Änderungen akzeptieren, aber Tech-Debt-Ticket einfordern.
Eskalationspfade
Die definierten Standards werden bei einer großen Zahl der gesamten Merge/Pull Requests helfen, schnell eine Einigung zu finden. Aber es wird immer Grenzfälle geben, ob eine Codestelle sicherheits- oder performance-relevant ist. Deshalb sollte ein Team besprechen, wie in diesen Fällen vorzugehen ist.
Ein möglicher Ablauf wäre zunächst ein (virtuelles) Gespräch zwischen Autor:in und Reviewer:in, um Argumente ausführlicher auszutauschen, gefolgt von einem Team-Call mit Mehrheitsentscheid, falls es vorher zu keiner Einigung kam.
Kommunikationsregeln
Zu guter Letzt einigen wir uns im Team auf Kommunikationsregeln für Kommentare in Reviews. Die Regelvorschläge kommen aus einem Brainstorming oder einer der umfangreichen Listen über eine Internetrecherche.
Beispiele:
- Wir kommentieren den Code, nicht den/die Autor:in.
- Wir erklären immer, warum wir eine Änderung vorschlagen.
- Wir verwenden Ich-Botschaften statt Du-Botschaften.
Weniger Konflikte in Code Reviews
Das Ergebnis des Workshops sind eine Menge von neuen Teamregeln für Code Reviews:
- Standard Reaktionen (akzeptieren/nicht-akzeptieren) je nach Motivation des Feedbacks
- Eskalationsregeln, wenn man sich nicht einigen kann
- Kommunikationsregeln für Feedback
Diese werden dem Team helfen, weniger und fairer in GitLab/GitHub zu diskutieren und zu weniger persönlichen Konflikten aus Code Reviews resultieren.
