Code Reviews: Got Code, Want Feedback

In Softwareentwicklungsteams hat sich die Durchführung von Code Reviews zur Erhaltung der Qualität und Wartbarkeit sowie zur Vermeidung von Fehlern und Problemen etabliert.

Wenn man sich nun etwas genauer mit Code Reviews beschäftigt, identifiziert man direkt die beiden grundlegenden Bestandteile: Den Code bzw. die Änderung am Code, der durch Entwickler:innen zur Verfügung gestellt wird, und das Feedback, das Reviewer:innen bezogen auf den Code hinterlassen.

Dieses Geben und Nehmen führt zwangsläufig zu Erwartungshaltungen auf beiden Seiten und soll im Folgenden unter die Lupe genommen werden.

Der Code

In der Regel bekommt man als Reviewer:in nur die konkreten Änderungen am Code – auch Diff genannt – zur Verfügung gestellt.

Dank Softwarelösungen wie z. B. GitLab, in denen sich ein Merge- oder Pull-Requests für solche Diffs verwalten lassen und so Reviewer:innen direkten Zugriff auf die geänderten Stellen im Code geben, kann man durch entsprechendes Syntax-Highlighting, variablen Kontext und verschiedene Ansichten zum Vergleich der Codestände angenehmer seiner Aufgabe nachgehen.

Wenn man sich dann den Umfang solcher Änderungen ansieht, kann man unterschiedliche Situationen vorfinden: Auf der einen Seite eine Vielzahl geänderter Dateien und auch geänderter Zeilen, die aber in Wahrheit auch nur eine strukturierte und tool-gestützte Umbenennung sind und daher wenig Risiko für Probleme bergen. Auf der anderen Seite kleine Änderungen, die wenige Stellen betreffen, aber eine hohe Signifikanz für die Funktion des Codes haben, z. B. die Änderung der Signatur einer zentralen Methode mit vielen Aufrufen. Selbstverständlich ist jede Abstufung zwischen diesen beiden Extremen ebenfalls möglich.

Noch mehr Herausforderungen

Und hier stolpert man auch schon über eine weitere Herausforderung als Reviewer:in und einen Nachteil des reinen Blicks auf die geänderten Codestellen: Nutzer:innen dieser Codestellen, auch indirekte oder solche, die sich schon vorher darauf bezogen haben, sind nicht sofort ersichtlich. Somit ist kein Rückschluss auf die Wirkung der Änderungen auf gewisse, nicht im Diff enthaltene Teile des Codes möglich. Dieser Umstand erfordert Ein- und Mitdenken bei Reviewer:innen, was über das reine Inspizieren der konkreten Änderungen des Codes hinausgeht.

Wie kann ich nun als Softwareentwickler:in den Reviewer:innen helfen?

Erfahrungsgemäß hat sich hierbei die Annahme „Vielleicht denkt und arbeitet mein:e Kollege:in ähnlich zu mir.“ bewährt und die Antwort auf die Frage „Wie bin ich bei der Entwicklung vorgegangen?“ als sinnvolle strukturelle Unterstützung erwiesen. Folgerichtig ist der einfachste Schritt, den Reviewer:innen konkrete Hinweise mitzugeben, über was sich bei der Entwicklung Gedanken gemacht wurde. Zum Beispiel welche Denkfehler selbst passiert sind und was explizite Designentscheidungen waren, die getroffen wurden.

Cascading Feature Branches können helfen

Etwas aufwändiger wird es, wenn das Vorgehen der Entwicklung in aussagekräftige Teile heruntergebrochen werden soll, die die Änderungen im Kontext einzelner Arbeitsschritte deutlich machen. Dafür können in Systemen zur Versionsverwaltung, beispielsweise Git, Commits oder auch „Cascading Feature Branches“ genutzt werden.

Cascading Feature Branches sind aufeinander aufbauende Feature Branches, zwischen denen ein überschaubar kleines Diff existiert und für die separat zu betrachtende Merge- oder Pull-Requests herhalten können. Die aufeinander aufbauenden Feature Branches sind auch ein Mittel, um bei potentiell zu großen oder unübersichtlichen Codeänderungen eine Aufteilung zu erreichen, die den Reviewer:innen einen besseren Überblick ermöglicht.

Generell ist es nie verkehrt, eine kurze Lösungsbeschreibung bzw. eine Erklärung zur Einordnung der Änderungen am Code zur Verfügung zu stellen. Ansonsten kann es zu Rückfragen der Reviewer:innen kommen, die vermieden werden könnten. Aber oftmals endet man als Entwickler:in, auch wenn man Informationen für Dritte zusammenfasst, bei Gedanken wie „Da hätte ich dem Code wohl einen Kommentar spendieren sollen.“ Aber es ist nie zu spät, das nachzuholen.

Das Feedback

Als Reviewer:in ist es die Aufgabe, Feedback zu den Codeänderungen an Entwickler:innen zu geben. Meist passiert dies asynchron und schriftlich, was aufgrund der Art der Aufgabenstellung von Vorteil ist. Doch bei genauerer Betrachtung ist das nicht immer ausreichend, wie weiter unten erläutert wird.

Das Feedback kann sich auf unterschiedliche Punkte der Änderungen beziehen. Zum einen sind es konkrete Stellen im Code, die durch Reviewer:innen kommentiert werden, zum anderen können es aber auch größere oder übergreifende Strukturen wie z. B. mehrere Klassen sein, zu denen es eine Anmerkung gibt. Schlussendlich kann es auch allgemeines Feedback zur gesamten Codeänderung geben.

Wenn hier von Feedback die Rede ist, sind damit aber nicht nur Verbesserungsvorschläge gemeint, die unterbreitet werden, sondern durchaus auch Rückfragen, die z. B. wegen Unklarheiten gestellt werden. Gerade diese Rückfragen haben eine besondere Funktionen und sind ein oft beobachtetes Phänomen in Code Reviews. Sie sind eine Form von wechselseitigem Lehren und Lernen, so dass beide Rollen in einem Code-Review mit einem Erkenntnisgewinn rechnen können. Dieser ist nicht nur auf den konkreten Code beschränkt, sonder kann auch generellerer Art sein.

Die asynchrone, schriftliche Kommunikation eignet sich, um Vorschläge für Anpassungen an den Änderungen zu machen oder auch um Fragen zu stellen. Jedoch stellt sich die Beantwortungen mancher Fragen, aber besonders die Diskussion zu Vorschlägen im persönlichen Gespräch – also synchron – als einfacher heraus und sollte dann auch auf diesem Weg erfolgen.

Die Erfahrung zeigt, dass bei Nutzung eines unterstützenden Systems für Reviews eine kurze Dokumentation nicht vergessen werden sollte, so dass andere, die evtl. mitlesen, das Ergebnis nachvollziehen können.

„Gutes“ Feedback

Was macht nun aber „gutes“ Feedback aus, das die Erwartungen von Entwickler:innen erfüllt, die dann basierend darauf weiter arbeiten wollen oder müssen. Die Kriterien, die erfahrungsgemäß erfüllt sein sollten, sind folgende:

• Ist das Feedback wertvoll, so dass z. B. durch die geforderte Änderungen wirklich etwas am Code oder der Software besser wird? Man könnte dies auch als Zweckmäßigkeit bezeichnen. Oder ist es nur sog. Bike-Shedding und der Schwerpunkt des Feedbacks liegt auf trivialen Aspekten anstatt auf vorhandenen und evtl. schwerwiegenden Problemen oder Entscheidungen (siehe auch „Law of triviality“ von C. Northcote Parkinson).
• Die Sachlichkeit wird durch den Bezug zum Code, dem Produkt, den Anforderungen und gemeinsamen Standards gewahrt. Vor allem ist das Feedback aber nicht von Gefühlen, Vorurteilen oder persönlichem Geschmack bestimmt.
• Inhaltlich, hin zu einer Lösungsorientierung, zeichnet sich das Feedback dadurch aus, konstruktiv, zielgerichtet und aussagekräftig zu sein. Kann ein:e Entwickler:in auf dieser Basis konkret die ersten Verbesserungen angehen, ohne Rückfragen stellen zu müssen? Ist ein Vorschlag durch die Reviewer:in direkt verständlich und kann mit wenig Unklarheit begegnet werden?
  

Insbesondere Trivialitäten (z. B. Code-Style) und offensichtliche Probleme (z. B. ein Syntaxfehler im Code) lassen sich vor dem eigentlichen Review durch Automatisierung – unter anderem Codeformatter, Linter und Tests – finden und teilweise auch automatisiert korrigieren. 

Entwickler:innen erhalten so frühzeitig Feedback, das sie berücksichtigen und vor der Einreichung des Codes zum Review einarbeiten können. Auf der anderen Seite kann den Reviewer:innen auf diese Art Lästiges und Formalisierbares abgenommen werden.

Von Code Style und Coding-Standards

Bei diesen Überlegungen helfen auch gemeinsamer Code-Style und Coding-Standards, die in entsprechenden Werkzeugen abgebildet oder konfiguriert werden können. Auch der Umstand, dass bestehende Funktionalität im Idealfall schon weitgehend durch Tests abgedeckt wird, entlastet Reviewer:innen zumindest mental. Durch diese technische Hilfen wird mehr Fokus und auch Zeit für das Feedback ermöglicht, um auf Wesentlicheres zu achten. Also Dinge, die durch Automatisierung eben nicht abgedeckt werden können.

Welche Aspekte dann Bestandteil des Feedbacks eines Code Reviews sein sollen, sollte man sich abhängig von der Situation, d. h. Team, Projekt und Automatisierungsgrad, vorher überlegen. Als Inspiration können folgende Punkte dienen:

• Sind die Qualitätsansprüche hinsichtlich Verständlichkeit, Wartbarkeit und Softwaredesign, d. h. Strukturierung des Codes, erfüllt?
• Sind die Anforderungen vollständig und korrekt umgesetzt?
• Ist die Benutzbarkeit der Software gegeben, d. h. sind Schnittstellen durch dritte sinnvoll nutzbar oder ist die Benutzeroberfläche entsprechend Vorgaben und Vereinbarungen umgesetzt worden?

Die unausgesprochene Klassifikation des Feedbacks

Irgendwann kommt man, zumindest bei der Nutzung von Merge- oder Pull-Requests an den Punkt: „Darf ich die Änderungen am Code mergen?“ Hier zeigt sich häufig, dass dies keine binäre Entscheidung ist und ebenso keine der Art „Das ist guter Code.“ versus „Das ist schlechter Code, der Änderungen bedarf.“ Vielmehr stellt sich heraus, dass hier eine, oftmals unausgesprochene, Klassifikation des Feedbacks gibt. Sie reicht von Tippfehlern in Variablennamen, die nicht unbedingt zu korrigieren sind, über Auffälligkeiten, die man sich für das nächste Softwaredesign-Meeting mitnehmen möchte, aber gleichzeitig nicht einer Übernahme der Änderungen im Weg stehen – bis hin zu gravierenden Problemen, die unbedingt gelöst werden müssen, bevor der Code seinen Weg in die Software findet.

Ein schönes Beispiel, das eine Analogie des Feedbacks zu Formen von Gestein findet, ist „Netlify’s Feedback Ladder“. Die Kommentare mit Feedback werden durch die Reviewer:innen mit Emojis oder Tags versehen, die für „Berg“, „Felsen“, „Stein“, „Sand“ und „Staub“ stehen. Das drückt die Klassifikation mit den Attributen „blockierend“ bzw. „nicht blockierend“ bezogen auf das Mergen auf der einen Achse und „braucht unmittelbares Handeln“, „erfordert Handeln in der Zukunft“, „benötigt in Zukunft Berücksichtigung“, „entscheide selbst“ bezogen auf die Dringlichkeit sowie Verpflichtung auf der anderen Achse aus.

Positives Feedback = Rauschen?

Zum Abschluss wäre da noch eine wichtige Sache, die Frage „Ist positives Feedback zu viel Rauschen im Code-Review?“ oder anders herum „Soll es nur Fragen und Änderungsvorschläge als Feedback geben?“

Wenn einer Reviewer:in eine Lösung gut gefällt oder eine elegante Strukturierung im Objektmodell gefunden wurde, dann ist es für die Bildung eines vertrauensvollen Verhältnisses unter Teamkolleg:innen sicherlich wichtig, auch das zum Ausdruck zu bringen. 

Außerdem hilft ernst gemeintes, positives Feedback ebenfalls beim Lernen. Und es tut gut.