Im Zuge der Vorstellung der Entwicklungstools von Atlassian hatten wir uns bereits mit Crucible beschäftigt. Im Artikel meines Kollegen David wurden dabei vor allem die Features von Crucible beleuchtet. Hier soll es nun darum gehen, welche Erfahrung wir bei unserer täglichen Arbeit mit dem Code-Review Tool von Atlassian gemacht haben.
Code Reviews ohne dediziertes Tool
Code Reviews kann man natürlich auch ohne ein dafür dediziertes Tool wie Crucible (oder Gerrit, oder Review Board, oder …) durchführen. Bei uns läuft das häufig so ab:
Ein Lead-Entwickler schaut sich die von einem Entwickler commiteten Änderungen am Quellcode an. Dabei hat er die IDE seiner Wahl offen um im Code navigieren zu können. Den zu begutachtenden Code ermittelt er über ein Diff über das Tool seiner Wahl (z.B. einer Erweiterung für die IDE). Möchte er dann Feedback zum Code geben, schreibt er entweder einen TODO-Kommentar direkt in den Code oder er nutzt dafür einen anderen Kommunikationskanal. Zum Abschluss des Reviews wird der Entwickler dann z.B. per Email über die Ergebnisse des Reviews informiert.
Ideal ist das aus verschiedenen Gründen nicht. TODOs im Code haben den Nachteil, dass oft vergessen wird diese zu entfernen, wenn das von ihnen beschriebene Problem behoben ist. Auch ist eine Diskussion am Code auf diese Weise äußerst müßig:
// TODO: I doubt this will work in production. Did you test the performance of this?
// Re: Had no problems w/ that on my machine.
// Re2: Ok. I was not precise enough: The problem here is the amount of users.
// I think there will be performance issues if there are more than just a few.
// Re3: Well I added some more users (100 now) and it still works fine.
// Re4: Well, by "more than just a few" I meant several thousands of users.
// Re6: Oops...
for (User user: getAllUsers()) {
loadGenerator.doSomeExpensiveStuff(user);
}
Ok, das Beispiel wirkt etwas gezwungen, aber ich denke es illustriert das Problem ganz gut, vor allem wenn man bedenkt, dass zwischen jedem "Re" ein Commit liegt. Wird stattdessen ein anderer Kommunikationskanal verwendet, ist es häufig recht umständlich die Problemstellen im Code zu benennen ("schau mal in Datei X auf Zeile Y"). Bei beiden Ansätzen verliert man außerdem schnell den Überblick über den Stand der Anmerkungen verschiedener Reviews. Wurde schon auf meine Anmerkungen reagiert? Wo sollte ich nochmal Feedback geben?
Code Reviews mit Crucible
Damit hätten wir also einige Werkzeug-bezogene Schwierigkeiten identifiziert, auf die man bei der Durchführung von Code Reviews stößt. Schauen wir uns also mal an, wie Crucible damit umgeht.
Da wäre zum einen das Problem der Diskussionen am Code. Hier vereint Crucible die Vorteile der beiden zuvor beschriebenen Methoden. Kommentare werden direkt an den jeweiligen Stellen im Code abgegeben und können auch direkt dort beantwortet werden. Da dies aber nicht in der Quellcode-Datei, sondern in einem externen Tool geschieht, wird der Quellcode dadurch nicht "verunreinigt". Weiterhin können Kommentare und Diskussionen auch auf Dateiebene oder sogar ohne direkten Bezug zu einem Artefakt erstellt werden. Zudem ist die Kommentar- bzw. Antwortfunktion recht intuitiv.
Das zweite Problem, das den Überblick über die Anmerkungen zu einem Review betrifft, löst Crucible, indem alle Kommentare zusammengefasst werden: Man erstellt immer erst ein Review in dessen Kontext dann alle Diskussionen erfolgen. Zu diesen Reviews gibt es dann einen Workflow und verschiedene Rollen (Ersteller, Moderator, Reviewer), die die am Review beteiligten Nutzer einnehmen. Je nach Rolle wird von einem Nutzer in einem Workflow-Schritt Input benötigt, oder eben nicht. Diese Information kann man sehr leicht auf dem Dashboard überblicken.
Beim Erstellen eines solchen Reviews wählt der Ersteller den zu begutachtenden Code aus und kann auch initial konkrete Fragen oder Hinweise an die Reviewer stellen. Hier sieht man schon einen wichtigen Unterschied zu dem von mir eingangs beschriebenen Ablauf: Nicht der Reviewer, sondern derjenige der gereviewt wird, hat die Initiative beim Erstellen des Reviews. Zwar ist es auch möglich, dass der Reviewer das Review selbst erstellt, allerdings muss man dafür den Ablauf etwas verbiegen, da der Moderator bei Crucible nicht gleichzeitig ein Reviewer sein kann. Hier trägt man dann am besten den Entwickler ein, dessen Code gereviewt wird. Außerdem erschien mir für dieses Szenario das Zusammenstellen des Codes weniger benutzerfreundlich, da die dazu zur Verfügung stehenden Möglichkeiten nicht für diesen Ansatz optimiert wurden. Hier fehlt z.B. die Möglichkeit alle Änderungen eines Nutzers ab einem bestimmten Zeitpunkt oder Commit auszuwählen. Daran sieht man, dass Atlassian sein Tool für agiles Vorgehen optimiert hat: Alle Entwickler arbeiten eigenverantwortlich an der Software und fordern wenn nötig selbständig ein Review ein. Glaubt man dem allgemeinen Hype, sind sie damit auf der richtigen Spur. Auch ich muss sagen, dass mir dieser Ansatz ganz gut gefällt.
Einen kleinen Kritikpunkt habe ich aber noch. Bei dem zu Beginn des Artikels beschriebenen Vorgehen habe ich erwähnt, dass zur Navigation im Code eine IDE verwendet wird. Diese benötigt man mit Crucible nach wie vor, da in der Code Sicht keine Navigation möglich ist. Hier wäre eine webbasierte Lösung ähnlich wie bei Docjar oder grepcode eine tolle Erweiterung.
Zusammenfassend kann man sagen, dass Crucible eine gute Ergänzung für die Tool-Landschaft zur Softwareentwicklung ist. Vor allem die Möglichkeit Diskussionen direkt am Code zu führen, ohne diesen anpassen zu müssen ist eine wirkliche Hilfe bei Code Reviews. Da wir allerdings gerade dabei sind Git und Stash zu evaluieren, stellt sich für uns die Frage, ob dieses Feature mit den Pull Requests von Stash nicht schon ausreichend abgedeckt ist. Wenn hier schon jemand Erfahrungen hat, würden wir uns über Feedback sehr freuen.
PS: Diese Woche wurde die Version 2.10 zu Crucible (und FishEye) veröffentlicht. Weitere Details gibt es im Produktblog von Atlassian.
