Vor einigen Monaten hatte ich über die Einführung von Code Reviews berichtet. Für uns ein wichtiger Schritt um die Qualität deutlich zu erhöhen. So war jedenfalls die Intention. Doch was hat sich inzwischen getan? Wurden die Code Reviews angenommen? Verwenden wir immer noch dasselbe Prozedere, oder mussten Änderungen am Prozess durchgeführt werden? Dieser Beitrag zeigt Probleme und mögliche Lösungswege auf.
Ausgangssituation
Im ursprünglichen Artikel habe ich erzählt, wie das Review abläuft:
Den Anfang hat das gemeinsame Zusammensetzung um einen PC gemacht. Danach werden sämtliche Changesets seit dem letzten Review durchgegangen. Ausgelassen wird nichts. Mittlerweile sind wir auf einen Beamer umgestiegen, da dies insgesamt ein wenig komfortabler sind. Der nächste Schritt könnte ein einschlägiges Tool sein, welches Kommentare etc. einfacher zulässt. So sicher bin ich mir hierbei allerdings noch nicht, da das “Zusammensitzen” das Team als solches wesentlich festigt und Ablenkungen minimiert werden. Es gibt kein blinkendes Skype, Lync, keine E-Mail-Benachrichtigungen und was sonst noch so alles aufblinkt.
Hat dies gehalten?
Veränderung durch Teamvergrößerung
Einige Wochen nach diesem Artikel kam die erste Herausforderung. Zu diesem Zeitpunkt hatten sich Code Reviews im Team ganz gut eingespielt und wurden meist täglich durchgeführt (zum Thema meist komme ich noch). Von quasi einem Tag auf den anderen wurde dann ein anderes Team mit meinem zusammengelegt. Schlagartig verdoppelte sich damit die Anzahl der Personen im Team. Eine harte Nuss.
Da zu viel an Veränderung meist nicht gut ist, haben wir versucht, den Prozess der Code Reviews beizubehalten. Und wir sind gescheitert. Zu viele Personen, zu viele Check Ins, drei Stunden Review. Das war dann doch eindeutig zu viel. Es musste eine andere Lösung her.
In Kombination mit der Zusammenlegung der Teams hatten sich auch einige andere Dinge geändert. So sind wir von Team Foundation Server auf Mercurial umgestiegen. Als Continuous Integration Server kommt nun Jenkins zum Einsatz. Im gleichen Atemzug ging die Installation von Review Board mit. Zusammen mit einigen weiteren Tools (siehe TortoiseHg, Extensions für die Einbindung des Review Boards, VisualHG usw.) schafften wir eine gut abgerundete Entwicklungsumgebung.
Aber wie war das jetzt mit dem Beamer?
Ein neuer Prozess
Es stellte sich also heraus, dass das Review im gesamten Team zwar durchaus Vorteile bietet, schlussendlich aber durch den immensen Zeitaufwand so nicht weiter verfolgt werden kann. Die Reviews sollten jedoch unbedingt weitergeführt werden, bringen sie doch zahlreiche Vorteile mit sich. Eine Änderung des Prozesses musste also her. Die Meinungen gingen klarerweise weit auseinander, von beibehalten über Zuteilung bis hin zu Auflassen war alles dabei. Worauf wir uns schlussendlich geeinigt haben:
- Täglich um 10:00 Uhr werden von allen Reviews durchgeführt.
- Jeder prüft die Anzahl der Review-Requests im Review Board und teilt sich selbst einer angemessenen Anzahl zu.
- Das Review wird im Laufe des Tages durchgeführt. Etwaige Fragestellungen/Probleme mit dem Implementierer besprochen und beim Review zur Aus-/Verbesserung gekennzeichnet.
- Änderungsanforderungen sind ASAP zu erledigen.
Ein Hinweis: Eine aufgekommene Idee war das “würfeln” der Zuteilung, da gerade bei der “Selbstauswahl” einige Schwierigkeiten entstehen können. Hierfür hatten wir zum damaligen Zeitpunkt jedoch keine ausreichend gute Lösung gefunden, wodurch diesbezüglich ein To Do entstand. Für konkrete Hinweise, Ideen hierzu bin ich natürlich dankbar, da dies ein äußerst nützliches Feature wäre.
Diese definierten Punkte fanden von allem Team-Mitgliedern Zustimmung und wurde somit sofort umgesetzt.
Eigenverantwortung ist ein Hund
Nun, Wochen später kann ein Resümee gezogen werden. Funktioniert dieser Prozess? Nun ja, … er funktioniert, optimal sieht jedoch anders aus. Die größte Herausforderung ist die im Prozess definierte Eigenverantwortung. Ein Teil der Entwickler reviewed tatsächlich täglich, ohne Erinnerung, ohne Hinweise. Es ist ihnen einfach wichtig bzw. nutzen diese “Chance” auch, ihre Implementierung durch eine zweite Person reflektiert zu bekommen. Bei einem anderen Teil funktioniert dies, trotz Serientermin und Erinnerung nicht so gut. Warum nicht?
- Vergessen: Der Klassiker. Trotz Erinnerung wird ganz gerne darauf vergessen. Die wahren Hintergründe kennt dann nur die Person selbst. Hier bedarf es laufender Erinnerungen und Hinweise.
- Umsetzungsstress: Durch die Umsetzung eines Punktes will zum genannten Zeitpunkt nicht unterbrochen werden. Verständlich, nachgeholt wird dies allerdings auch nicht, sondern vielmehr auf den nächsten Tag verschoben.
- Fehlende Wichtigkeit: Vielfach werden andere Tasks als wichtiger angesehen. Punkte die zum Review anstehen werden teilweise als abgeschlossen angesehen, obwohl sie das nicht sind. Hier müssen Prioritäten klar gesetzt werden. Gerne auch wiederholt.
- Fehlende Kritikfähigkeit: Eigentlich nur eine Vermutung, da eine Bestätigung nur schwer zu erhalten ist. Vielfach sehe ich aber das Problem, dass eigener Code nicht gerne der “allgemeinen Bewertung” zugeführt wird, um den eigenen “Status” nicht zu gefährden. Soll heißen: Die anderen könnten ein negatives Bild der eigenen Kenntnisse erhalten.
- Einfache Changesets bevorzugt: Wer es sich aussuchen kann, nimmt die einfachen Changesets. D.h. idealerweise nur eine betroffene Datei, keine komplexeren Patterns/Implementierungen etc. Als Resultat bleiben größere Changesets über und werden nicht, oder nur von einem ganz kleinen Kreis reviewed.
Als Fazit kann man sagen: Teile der Entwickler sehen einen großen Mehrwert in Reviews und nutzen dieses Mittel aktiv, um eigenen Code und den anderer Entwickler verbessert zu. Wo dieses Verständnis fehlt, funktioniert es nicht. Es muss ganz klar vermittelt werden, dass Reviews einen Mehrwert bieten und der Verbesserung der Software, als auch der eigenen Kenntnisse dient. In der Tat, ein schwieriges Unterfangen.
Der Hirtenhund hat Urlaub
Irgendwann geht auch der Team Lead auf Urlaub. Wie sieht es dann mit Code Reviews aus?
<
blockquote> Der Idealfall: Es läuft. Ohne Zutun.
<p><strong>Die Realität</strong>: Die Reviews verlaufen im Sand. Diejenigen, die vorher ohne jegliches Zutun Reviews durchgeführt haben, tun dies auch, wenn die Rute nicht ins Haus steht, viele Punkte bleiben jedoch ungesichtet/bewertet.</p> </blockquote>
Was genau Abhilfe schafft, weiß ich noch nicht genau (für Hinweise, Anregungen bin ich dankbar). Vorerst gibt es ständige Erinnerungen an die jeweiligen Personen, Reviews durchzuführen.
Stress durch Releases
Ein ähnliches Verhalten habe ich bei anstehenden Releases beobachtet. Steht ein größeres Release ins Haus, dann sollten – aus meiner, vielleicht etwas naiven Sicht – Reviews verstärkt stattfinden (bzw. noch genauer als normal durchgeführt werden). Gerade unter Stress erhöht sich die Wahrscheinlich von Schlampigkeit. Gerne werden Wege abgekürzt, Tests vernachlässigt. Da ist es unerlässlich, dass viele dieser “Fehler” durch Reviews abgefangen werden. Aber auch hier zeigt sich: Es wird an allen Ecken und Enden gespart. Vielmals wird es vorgezogen, auf einen Bugreport des Test-Teams zu warten und dann das Problem zu fixen, als es sofort anzugehen.
Ein wenig Statistik
Was wäre ein Bericht über das (Nicht)Funktionieren von Code Reviews ohne Daten. Nachfolgend eine (wirklich) kleine Statistik, die sicherlich einige Punkte außer Acht lässt, aber doch einige meiner Aussagen bekräftigt.
Hinweis: August und September sind voll gerechnet. Oktober ist natürlich nicht vollständig enthalten, da zum Zeitpunkt dieses Artikels erst die Mitte erreicht ist. Dennoch können einige valide Aussagen abgeleitet werden.
Die Zahlen sind nun recht nett anzusehen, bedürfen aber einiger Erklärungen, um tatsächlich sinnvoll zu sein bzw. ein Bild abzugeben. Aber der Reihe nach. Zur Bewertung kamen alle Review Requests, die via Review Board durchgeführt wurden. Reviews, die davor in großer Runde vorgenommen wurden, sind hier nicht berücksichtigt.
Allgemeine Review Zahlen: Insgesamt wurden 350 Review Requests abgegeben. 72 davon wurden mit einem To Do für den Implementierer versehen, mussten also nochmals angegriffen und verbessert werden (das sind immerhin 21% aller Requests). 9 der 72 notwendigen Anpassungen/Verbesserungen mussten mehrfach aktualisiert werden, bis der Code akzeptiert wurde.
Reviews nach Monaten: Der Startmonat verlief alles in allem ganz gut. Auffallend ist die doch wesentlich geringere Anzahl an Requests im September. Im Oktober stieg die Anzahl wieder sehr stark an (obwohl erst die Hälfte vorbei). Des Rätsels Lösung: Ich als Team Lead war im September für zwei Wochen auf Urlaub. Vorher fanden entsprechende Erinnerungen statt, danach ebenfalls.
Hinweis: Eine weitere Maßnahme besteht auch darin, darauf zu achten, dass möglichst jedes Check In im Review Board landet. Hier gibt es noch ein gewaltiges Potential.
Fixes nach Monaten: Hier eine Aufschlüsselung danach wie viele Punkte in den jeweiligen Monaten “korrigiert” werden mussten. Auch hier sank die Zahl im September doch recht stark ab bzw. dürfte für Oktober wesentlich höher werden (wie gesagt, erst die Hälfte rum).
Verbesserungen
Aus diesen Erfahrungen lassen sich einige Verbesserungen ableiten. Manche mit konkretem Task, andere benötigen noch Ideen, Gespräche und anderweitige Erfahrungsberichte. Here we go:
- Würfeln. Anfangs sind wir – vermutlich – zu schnell über die Idee mit dem Würfeln und automatischen Zuteilen von Review Requests an Personen hinweg gegangen. In der Realität sollte dies aber einer der essentiellsten Punkte sein. Unabhängig der Komplexität eines Changesets werden die Punkte zugeteilt. Anstatt abgehandelte Reviews nach dem “Wer hat’s gemacht” zu durchsuchen, kann dies anhand der offenen Requests festgestellt werden.
- Steigerung Akzeptanz. Einfacher gesagt als getan. Immer wiederkehrendes Wiederkauen der Vorteile, das Austreiben der Angst vor Fremdbewertung, das Formen des Teamgedankens sind definitiv notwendig um das “System” wirklich am Leben zu halten. Damit verkommt man schnell einmal zum Don Quijote der Code Reviews.
- Automatisches Review Posting. Reviews werden aktuell manuell durch den Benutzer angefordert. Pro Check In muss dieses über Command Line getriggert werden. Dadurch landen viele Check Ins nicht am Review Board.
Grundsätzlich hätte ich noch ein paar Punkte in peto, allerdings sollten die obenstehenden für die nahe Zukunft ausreichend sein und stellen – meiner Meinung nach – die wichtigsten Verbesserungen dar.
Feedback
Gerne freue ich mich über Feedback, zumal zahlreiche meiner Leser auf diesem Gebiet sehr bewandert sind und das eine oder andere Problem vielleicht schon gelöst haben. Vielleicht stellen sich aber auch einige meiner Gedankengänge als fehlerhaft heraus und die Lösung wäre so viel einfacher. Zum Schluss aber noch eine Frage, die ich mir laufend stelle, jedoch bisher noch keine Antwort darauf finden konnte:
Gibt es eine Möglichkeit die Qualität eines Reviews festzustellen?