Code-Reviews sind wie Fahrrad fahren: Hat man es einmal gelernt, verlernt man es nicht mehr. Einmal gelernt, fahren wir außerdem alle auf fast die gleiche Weise Fahrrad.
Diesen Irrglaube, dass jeder weiß wie Code-Reviews durchzuführen sind, findet man in vielen Entwicklerteams. Egal ob sie gerade erst mit Code-Reviews angefangen haben oder ob sie diese bereits länger einsetzen.
Doch was gilt es bei Code-Reviews zu beachten? Welche Tricks gibt es, um häufig aufkommende Probleme gar nicht erst entstehen zu lassen? Dieser Artikel zeigt dir 10 Punkte auf, die deine Code-Reviews wirksamer machen werden.
Es gibt verschiedene Arten von Code-Reviews. Neben Walkthroughs, Inspections und Pull-Request-Reviews kann man auch Pair-Programming als Code-Review ansehen. Dieser Artikel beschränkt sich auf die klassischen Reviews von Pull- oder Merge-Requests, je nachdem ob du ein Github- oder Gitlab-Liebhaber bist beziehungsweise was in deinem Projekt gerade angesagt ist.
Bevor wir zu unserem-10-Punkte-Plan kommen, werfen wir einen Blick auf Pro- und Kontra-Argumente, die bei jeder Diskussion um die Einführung von Code-Reviews aufkommen werden.
Auf der Kontra-Seite wird in Diskussionen häufig Folgendes genannt:
Auf der Pro-Seite sind folgende Argumente erwähnenswert:
Die nachfolgenden 10 Punkte sind eine Sammlung von Hilfestellungen und Best Practices, die wir über verschiedene Projekte hinweg gesammelt und ausgiebig getestet haben. Sicherlich werden auch für dich einige Punkte dabei sein, die deinen Code-Review Alltag verbessern werden.
Das Feature ist nach mehrtägiger Entwicklung fertig und der Pull-Request kann erstellt werden. Eigentlich solltest du selbst vorab prüfen, ob nicht noch kleinere Fehler wie Formatierung, To-dos oder überflüssige Kommentare vorhanden sind. Diese Aufgabe wird leider allzu oft dem Reviewer überlassen.
Diese Unachtsamkeiten hätten dem Ersteller vor dem Code-Review auffallen sollen.
Besonders in erfahrenen Entwicklerteams geht durch solche Pull-Requests viel Zeit und kognitive Aufmerksamkeit für Kleinigkeiten verloren. Solche Minifehler findest du selbst viel effizienter. Lieferst du einen Pull-Request ohne diese Minifehler, kann sich der Reviewer auf die wichtigen Fragen wie Architektur, Tests und Anforderungen konzentrieren.
Eine deiner Kernaufgaben bei Code-Reviews ist es, zu überprüfen, ob die Anforderungen gemäß der Akzeptanzkriterien der User-Story umgesetzt sind. Eigentlich wird diese Aufgabe dem Product-Owner zugeschrieben. Die konkrete Umsetzung der jeweiligen Anforderung im Code kann allerdings oft nur ein Entwickler umfassend überprüfen.
Daher lohnt es sich, vor dem eigentlichen Code den Code der Tests zu überprüfen, denn dort sollten die Anforderungen festgehalten und überprüfbar gemacht worden sein.
Dieses Vorgehen stellt die Tests in den Fokus und wendet damit das Konzept von Test-Driven Development auch auf Code-Reviews an. Solltet ihr noch nicht testgetrieben entwickeln, so wird es spätestens jetzt Zeit dazu. Damit stellt ihr in jedem Fall sicher, dass es zu jedem Code-Review auch Tests zum Überprüfen gibt.
Nicht selten fühlen sich Entwickler durch Feedback persönlich angegriffen. Das liegt zum einen oft daran, dass man sogar völlig objektive Kritik leicht auf die eigenen Fähigkeiten bezieht. Zum anderen liegt es an einer Art von Kritik, die nicht den Code kritisiert, sondern den Coder.
Folgendes Beispiel illustriert eine völlig unnötige persönliche Kritik:
Beispiel für persönliche Kritik.
Der erste Satz ist an dieser Stelle überflüssig, da er eine Schwäche des Entwicklers kritisiert, verallgemeinert (“always”) und keine Hoffnung auf einen Lerneffekt erkennen lässt. Der wesentliche Inhalt des Kommentars ist ausreichend im zweiten Satz enthalten und hätte als Kommentar ausgereicht, um den Fehler im Code aufzuzeigen.
In den meisten Teams hat der Reviewer in einem Code-Review das letzte Wort und erwartet, dass alle seine Änderungsanforderungen umgesetzt werden. Damit entscheidet er, ob und wann ein Pull-Request fertig ist und die Änderungen in den Master aufgenommen werden.
Die Menge der Kommentare ufert nicht selten aus.
Kehrt doch diese Verantwortung einfach einmal um: Der Ersteller eines Pull-Requests kann entscheiden, welche Anforderungen des Reviewers er direkt umsetzt und wann ein Pull-Request abgeschlossen ist. Damit ändert sich seine Sichtweise auf den Pull-Request: „Ich bin selbst verantwortlich für die Umsetzung unserer Qualitätsansprüche.“
Dies führt meistens nicht wie befürchtet zu weniger, sondern zu mehr Sorgfalt bei der Bearbeitung von Pull-Requests.
Nach zwei Wochen Entwicklung in deinem eigenen Branch hast du das Feature nun umgesetzt. Der dazugehörige Pull-Request hat eine Größe von mehreren Dutzend geänderten Dateien und mehreren Hundert Zeilen geänderten Codes.
Extrembeispiel für einen riesigen Pull-Request.
Solche Pull-Requests sind auch von erfahrenen Entwicklern nur mit sehr viel kognitivem und zeitlichem Aufwand zu überprüfen. Zusätzlich lassen sich tiefgreifende Änderungen am Design des Features an dieser Stelle oft nur noch mit sehr hohem Aufwand umsetzen.
Zerlege dein Feature lieber in kleine Teile! Jeder dieser Teile kann in einem eigenen Pull-Request überprüft werden und die einzelnen Teil bauen dann aufeinander auf. Dies mag auf den ersten Blick zwar einen Mehraufwand bei der Entwicklung bedeuten, lohnt sich aber durch den geringeren Aufwand für Code-Reviews und Anpassungen. Vor allem ermöglicht dir dieses Vorgehen ein frühes Feedback für deine Entwicklung einzuholen. Dadurch kannst du auf Änderungswünsche rechtzeitig eingehen. Langfristig sinkt damit der Aufwand für die Entwicklung neuer Features.
Ein Reviewer schaut sich deine Änderungen an und schlägt eine Verbesserung von Codezeilen vor, die in deinem Pull-Requests gar nicht geändert wurden. Schnell kommt die Frage auf: „Wer soll diese Verbesserung denn nun umsetzen? Ich hatte doch mit diesem Code gar nichts zu tun.“
Statt euch mit dieser Frage zu beschäftigen, sollte euch klar sein, dass ihr als Team für alle Teile des Codes verantwortlich seid. Somit ist die Frage nicht, wer diese Verbesserung umsetzen soll, sondern eher wann das geschehen kann. Entweder kannst du sie selbst innerhalb deines Features mit umsetzen oder ihr erstellt eine neue Story dafür, weil es zum Beispiel eine technische Schuld ist.
Über Multitaskingfähigkeit wird viel diskutiert. Im Entwickleralltag zeigt sich jedenfalls, dass sich Entwickler häufig auf nur eine einzige Abstraktionsebene im Code konzentrieren können. Abstraktionsebenen sind unter anderem Formatierung, Lesbarkeit und Verständlichkeit, Architektur und Design, Sicherheit oder die Umsetzung der Anforderungen. Idealerweise betrachtest du den vorliegenden Code im Review auf all diesen Ebenen zugleich und gleichzeitig aus der Vogelperspektive.
Das ist natürlich unrealistisch. Daher lohnt es sich, dass du dir den Code bewusst mehrfach anschaust. Dabei machst du dir klar, welche Abstraktionsebene du gerade im Fokus hast. Wir erinnern uns an Punkt 1: „Reviewe dich selbst.“ Dies trägt dazu bei, den Aufwand für die Ebene Formatierung so gering wie möglich zu halten.
Du hast von deinem Reviewer mehrere Kommentare erhalten und setzt diese nun Stück für Stück um. Häufig versäumt man es, sich an dieser Stelle zu fragen: „Ist diese Änderung eigentlich sinnvoll?“ Versäumt es nicht, an dieser Stelle über die Änderungen zu diskutieren und euch über die Implikationen auszutauschen! Ihr werdet dabei beide etwas lernen, denn auch wenn eine Änderung nicht wirklich sinnvoll ist, ist es das Gespräch darüber aber allemal. Übrigens: Im persönlichen Gespräch ist das zielführender, als Pingpong in Code-Review-Tools zu spielen.
Viele Aspekte eines Reviews kann eine Maschine viel besser überprüfen als du: Formatierung, ungenutzte Variablen, überflüssige Debug-Ausgaben und Code-Coverage lassen sich beispielsweise sehr gut automatisiert durch Tools wie SonarQube und Linter überprüfen. Viele Teams haben diese Tools zwar bereits im Einsatz, allerdings werden die generierten Berichte nur selten direkt im Code-Review-Tool dargestellt.
Nach einem Review noch zusätzlich den SonarQube-Bericht auf einer weiteren Webseite zu überprüfen, gerät schnell in Vergessenheit. Daher können viele dieser Tools direkt oder mittels Bots als automatisierte Reviewer direkt in einen Review-Prozess eingebunden werden. Bereits direkt nach der Einbindung dieser Informationen, wird dies euren Review-Prozess erleichtern, da ihr euch auf die wesentliche Dinge konzentrieren könnt, die eine Maschine eben nicht erledigen kann.
Trotz Automatisierung wird es weiterhin Aspekte eines Reviews geben, an die du als Reviewer bei jedem Review denken musst. Fordere dein Gedächtnis an dieser Stelle nicht zu sehr heraus!
Sammle stattdessen Themen wie zum Beispiel SOLID-Prinzipien, Sicherheits- und Logging-Anforderungen zentral in einer Checkliste als Teil der Definition of Done. Diese Checkliste sollte während eines Reviews immer präsent sein, zum Beispiel als Teil jeder User-Story oder als Kommentar innerhalb des Code-Review-Tools.
Dieser Artikel sollte dich dazu motivieren, deinen eigenen Code-Review-Prozess einem Review zu unterziehen. Einige der 10 vorgestellten Punkte wirst du sicherlich umsetzen können, um euer Review wirksamer zu gestalten. Code-Reviews solltet ihr aber nicht nur als Maßnahme zur Fehlervermeidung ansehen, sondern als unschätzbares Werkzeug um effizient Wissen und Know-how im Team zu verbreiten. In diesem Sinne, viel Spaß bei deinem nächsten Code-Review!