Category

Neulich bei der Code-Review in einem Angular-Team (ein fiktiver Dialog):

READ MORE

Alle finden Review toll, jeder will es machen … aber was, wenn es ernst wird?

Wenn in einer Runde von Software-Entwicklern die Frage gestellt wird, wie man zu Code Review steht, sind diese sich meist einig. Sie antworten mit «Finde ich gut, machen wir in unserem Team» oder «Finde ich gut, würde ich gerne machen».

Stellen sich Software-Entwickler wirklich gerne mit heruntergelassener Hose vor das Team?

Die Antwort lautet, jein…

Bis ein Entwickler sich an den Review-Prozess gewöhnt hat, dauert das meist eine Weile bis er Reviews nicht mehr mit Angstschweiss und Herzklopfen verbindet. Wer hört schon gerne Kritik oder offenbart sich dem anderen, dass er einen auf seine Fehler hinweist? Aber wenn Reviews einmal zum Alltag gehören, werden die Feedbacks sehr geschätzt. Denn jedem Entwickler ist von Anfang an klar, jeder Fehler, der noch im Entwicklungsprozess gefunden wird, ist günstiger und weit weniger peinlich, als wenn er draussen im Feld auftaucht.

Review Arten

Es gibt viele unterschiedliche Arten, wie das Code Review in den aktuellen Entwicklungsprozess eingebaut werden kann. Es muss entschieden werden wie ein Review durchgeführt werden soll. Reviews können zum Beispiel im 4-Augen Prinzip, Team-Review oder asynchron über ein Tool durchgeführt werden. Das Team sollte sich auch einigen, welcher Code in einem Review überprüft werden soll. Soll jeder Checkin/Push, jede Neuimplementierung oder jeder Bug überprüft werden? Wer aus dem Team ist für welches Review zuständig bzw. wer kann diesen Code im Zusammenhang am besten beurteilen? Was soll in einem Review geprüft werden? Sind die Akzeptanzkriterien der Anforderung, das Naming von Klassen/Methoden/Parametern, die Testbarkeit, die Erweiterbarkeit, die Wartbarkeit, die Struktur, die Methoden-/Klassengrösse und/oder Error-Handling Teil des Reviews?

Was ist drum herum noch zu beachten?

Um den Review-Prozess herum gibt es aber noch andere Punkte, die das Review beeinflussen. Wenn Coding-Guidelines vorhanden sind, können und sollten diese über Tools geprüft werden und nicht Teil eines Reviews sein. Architektonische Entscheidungen sollten vor der Implementierung besprochen werden und dürfen nicht erst beim Review auffallen. Wenn in einer Erweiterung schon vorhandener Code verwendet wird, kann dieser in einem Review kritisiert werden? Gibt es hierfür einen anderen Prozess oder wird das innerhalb dieser Erweiterung verbessert?
Alle Entscheidungen haben pro und contra, deswegen sollte das Team entscheiden, welcher Weg der passendste ist. Dem Team muss aber bewusst sein, dass wenn es sich zum Beispiel für asynchrones Review entscheidet, diese Entscheidung nicht grundsätzlich das 4-Augen Review ausschliessen muss.

Beispiel eines gelebten Review Prozesses

In meinem aktuellen Projekt haben wir seit einiger Zeit einen neuen Review-Prozess eingeführt mit der Unterstützung von Bitbucket. In diesem Prozess haben wir definiert, dass es für jede Code-Änderung ein Issue in Jira geben muss, aus dem dann ein eigener Branch erstellt werden kann. Bei einer Code-Änderung kann es sich um ein Feature, einen Bug oder um Refactoring handeln. Wir haben uns für asynchrone Reviews mittels Bitbucket entschieden, da die Reviews zeitunabhängig ablaufen können. Dennoch versuchen wir, Reviews zeitnah durchzuführen um die Stories schnellstmöglich dem Testprozess übergeben zu können. Die Tools Jira und Bitbucket unterstützen uns in diesem Prozess optimal. Per Pull Request in Bitbucket werden die gewünschten Reviewer aus dem Entwickler- und Test-Team eingeladen die Änderungen zu prüfen. Die Änderungen werden wie in gewöhnlichen Merge-Tools dargestellt und es können Kommentare und Tasks für den Autor erfasst werden. Der komplette Pull Request kann von einem Reviewer akzeptiert oder abgelehnt werden.

Für mich bringt dieser Prozess Vor-und Nachteile mit sich.

Pro und Contra

Vorteile sind für mich in diesem Ablauf, dass
– jede Code-Zeile nachvollziehbar kommentiert werden kann
– das Review unabhängig von Autor und anderen Reviewern durchgeführt werden kann
– das bisherige Feedback für nachfolgende Reviewer ersichtlich ist, so dass widersprüchliche Feedbacks und Meinungsverschiedenheiten der Reviewer vorher abgeklärt werden können und nicht direkt an den Autor zurückgegeben werden
– jeder Code im Masterbranch durch dieses Review ging
– jeder Checkin/push zu einem eigenen Branch und Issue in Jira gehört -> Nachvollziehbarkeit verbessert sich
– das Know-How (Implementation sowie Anforderungen) von neuen Features an weitere Entwickler im Team übertragen wird

Jedoch gibt es auch Punkte, die beachtet und möglicherweise anderweitig gelöst werden müssen:
– geschriebene «Kritik» wird oft falsch verstanden oder persönlich interpretiert
– geschriebenes Feedback ist schwieriger verständlich
– ohne festen Termin werden Reviews gerne nach hinten geschoben
– der Blick für das Ganze fehlt möglicherweise oder wird nicht betrachtet und Zusammenhänge werden nicht bedacht und können vergessen gehen
– oft wird nur der neue/veränderte Code geprüft
– der Happy Path kann sehr schnell geprüft werden, jedoch können mögliche Nebeneffekte ebenfalls schnell übersehen werden
– endlos Diskussionen bei Meinungsverschiedenheiten über guten Code, wo hört Geschmack auf und wo fängt Design (Clean Code z.B.) an?

Konfliktpotential

Bei einem Review sollten wir auch im Hinterkopf haben, dass ein Review Kritik, Verbesserungsvorschläge und Meinungsverschiedenheiten beinhaltet, worauf jeder Mensch unterschiedlich reagiert. Um Konflikte zu vermeiden sollten Autor und Reviewer auf Betonung und Wortwahl achten um das Gegenüber nicht zu verletzen. Aussagen wie «Mach das so» ohne Begründung und «das ist dreckig gelöst» sollten besser umschrieben oder in Fragen umgewandelt werden, warum das Problem nun so gelöst wurde. Grundsätzlich ist es immer gut, eine Begründung für das Feedback anzugeben. Bei einer Meinungsverschiedenheit, die in eine längere Diskussion ausartet, können beide sich die Frage stellen, ob der aktuelle, diskutierte Punkt den Code wirklich verbessert oder einen Bug beinhaltet – oder ob es nur eine weitere Lösung wäre, die aber ausser Zeitkosten keinen weiteren Benefit bringt.

Auch wenn manche Reviews an den Nerven zerren, überwiegen für mich die Vorteile. Neben den oben genannten Pros, lerne ich zusätzlich die Denkweise und Arbeitsweise meines Teams besser kennen. Ich kann mich im Projekt weiterentwickeln, denn nicht nur mein Code wird überprüft, sondern ich sehe auch Lösungsansätze der anderen und kann diese in meiner nächsten Aufgabe mit einfliessen lassen.

Ich hoffe, dass in meinem nächsten Projekt wieder ein so gut ausgebauter Review-Prozess vorhanden ist. Denn auch wenn ein Review nicht in erster Linie Bugs findet, verbessert sich der Code und das Team lernt dazu.