Ratgeber

So gehen Googles Entwickler bei Code-Reviews vor

Pair-Programming soll den Wissensaustausch zwischen den Programmierern steigern. (Bild: Pivotal Software)

Bei Google – und in den meisten anderen Tech-Unternehmen auch – ist es Best Practice, dass vor dem Merge in den Master-Branch eine Code-Review gemacht wird – und wir können uns einiges von den Prinzipien des Tech-Giganten abgucken.

Eine Code-Review nennt man den Prozess der Abnahme eures Codes durch eine geeignete dritte Person. Klassischerweise geschieht das, nachdem ihr fertig mit dem Bauen eines Features seid und alle Tests erfolgreich waren. Gemacht wird eine Code-Review, um die Qualität der Codebase sicherzustellen. Entwickler neigen, wie Angehörige vieler anderer Berufszweige auch, oft zu einer Art Betriebsblindheit. Außerdem gilt in der Entwicklung, wie in vielen anderen Bereichen auch: Vier Augen sehen mehr als zwei.

Zielsetzungen

Oberstes Ziel der Praxis ist es, eine kontinuierliche Verbesserung der Codebase des Unternehmens herbeizuführen. Um das zu erreichen, müssen Kompromisse eingegangen werden. Die Zielvorgabe darf euch als Entwickler nicht davon abhalten, überhaupt Code zu schreiben. Seid ihr durch zu starre oder zu hoch gesteckte Vorgaben und Ziele gehemmt, Änderungen am Programm vorzunehmen, werdet ihr auch keine Fortschritte verzeichnen. Auf der anderen Seite muss der Abnehmer eures Codes sicherstellen, dass die Qualität der Codebase nicht schleichend immer schlechter wird. Vor allem unter Zeitdruck können schnell kleine Fehler und Ungenauigkeiten passieren. Wer weiß, dass er seinen Code direkt im Anschluss einer dritten Person zur Durchsicht geben muss, achtet eher darauf, ihn auch für andere – und somit auch sein zukünftiges Ich – nachvollziehbar zu schreiben.

Damit der Code bei Google konsistent und wartungsarm bleibt, muss der Abnehmer des Codes eine Reihe von Dingen prüfen:

  • Passt das Design des Codes zum Rest der Applikation? Mit Design ist hier nicht das Design des Frontends gemeint, sondern technische Entscheidungen darüber, wie die Anwendung programmiert werden soll.
  • Verhält sich der Code so, wie von seinen Autoren vorgesehen? Funktioniert das aus Nutzersicht?
  • Könnte man das einfacher schreiben? Wie lesbar und verständlich ist der Code für Außenstehende, die eventuell in Zukunft daran weiterarbeiten müssen?
  • Gibt es automatisierte Tests? Sind diese sinnvoll und erfolgreich?
  • Wurden Variablen, Klassen und Methoden sinnvoll benannt?
  • Sind die Kommentare sinnstiftend und verständlich?
  • Folgt der Code den Style-Guides?
  • Wurden die Docs aktualisiert?

Entstand euer Code im sogenannten Pair Programming und ihr wärt beide qualifiziert, eine Code-Review durchzuführen, gilt der Code als abgenommen.

Eine alternative Vorgehensweise zur Code-Review über GitHub ist die persönliche Code-Review – bei der der Abnehmer eures Codes euch Fragen dazu stellt. Generell stellt Google die Regel auf, dass der Code akzeptiert werden muss, sobald er eine Verbesserung für die Gesamtheit der Codebase darstellt, auch wenn er nicht perfekt ist. Es gibt nämlich keinen perfekten Code. Keine Regel ohne Ausnahme: Wenn euer Code ein Feature zur Codebase hinzufügt, das der Abnehmer gar nicht in seinem System haben möchte, kann er den Merge-Request natürlich ablehnen, auch wenn eure Arbeit ansonsten den Anforderungen entspricht. Ein Delay für ein gewünschtes Feature aufgrund fehlender Perfektion wird hingegen nicht akzeptiert, Code ist nie perfekt. Einen entsprechenden Kommentar darf der Reviewer aber trotzdem hinterlassen, versehen mit einer Notiz, dass es sich hierbei um einen Kommentar handelt, der ignoriert werden darf.

Code-Reviews als nützliches Mentoring-Instrument

Die Kommentare in einer Code-Review sind eine nützliche Learning-Ressource. Kommentare, die Entwicklern helfen sollen, ihre Coding-Skills zu verbessern, aber nicht unbedingt zur Einhaltung der Standards aufgelöst werden müssen, sind grundsätzlich erlaubt und erwünscht, sollten dann aber mit einem entsprechenden Präfix (zum Beispiel: „Nit:“) versehen werden.

Lob

Eine Code-Review konzentriert sich per Default auf die weniger gut gelösten Teile eines Programms – sie ist dazu da, Fehler zu eliminieren und die Qualität der Codebase zu wahren. Wenn ihr als Reviewer eine besonders elegante Lösung für ein Problem im Code eines Entwicklers findet: Lasst ein Lob da. Ein kurzer Comment wie „nice!“ kostet keine Zeit und reicht aus, um den Entwickler in guten Arbeitsweisen zu bestärken. Das ist vor allem aus Mentoring-Sicht wichtig und wertvoll.

Grundsätzlich folgt eine Code-Review immer den folgenden Prinzipien

Technische Daten und Fakten haben mehr Gewicht als persönliche Präferenzen. Geht es um Stil, ist der Styleguide das oberste Gebot. Nicht im Styleguide explizit definierte Styles sind persönliche Präferenz – sollten jedoch konsistent mit bereits vorhandenem Code sein. Software-Design ist so gut wie nie einfach eine Stilfrage oder persönliche Präferenz. Wenn es innerhalb der dem Design zugrunde liegenden Prinzipien mehrere valide Optionen gibt, sollte der Reviewer grundsätzlich die Wahl des Entwicklers anerkennen – solange sie konsistent mit dem Stil der bereits vorhandenen Codebase ist. Einzige Ausnahme: Der Reviewer kommt zu dem Schluss, dass die allgemeine Stabilität der Codebase durch den Stil des Autors verschlechtert wird.

Konflikte lösen

Wenn es über eine Code-Review zu einem Konflikt kommt, sollten Entwickler und Reviewer grundsätzlich versuchen, mithilfe der Richtlinien zu einer Einigung zu kommen. Wenn sich das als besonders schwierig erweist, sollten Entwickler und Reviewer in einem persönlichen Gespräch versuchen, eine Lösung zu finden und sich zu einigen. Das funktioniert meistens besser, als sich über Kommentare zu streiten. Findet ein persönlicher Austausch statt, sollte das Ergebnis in Form eines Kommentars in der Change-List festgehalten werden.

Für den Fall, dass die Situation sich so nicht auflösen lässt, wäre die nächste Stufe die der Eskalation (steht wirklich so im Dokument: If that doesn’t resolve the situation, the most common way to resolve it would be to escalate). Der Eskalationspfad ist meistens der einer breiteren Team-Diskussion, in der ein Teamleiter seine Meinung abgibt, ein Maintainer des Codes die Entscheidung trifft oder ein Engineering-Manager aushelfen soll. Geht gar nicht: Ein Stück Code nicht zu mergen, weil Autor und Reviewer sich nicht einigen können.

Fazit: Eigentlich geht es bei einer Code-Review vor allem darum, basierend auf Richtlinien wie sie zum Beispiel innerhalb eines Styleguides definiert sind, die beste Entscheidung für eure Codebase zu treffen. Das erfordert analytisches Geschick, Kompromissbereitschaft und die Fähigkeit, jeweils Kritik zu äußern und anzunehmen.

Mehr Details zum Thema Code-Review bei Google findet ihr in Googles Code Review Developer Guide.

Vielleicht auch interessant für dich: 

Zur Startseite
Bitte beachte unsere Community-Richtlinien

Wir freuen uns über kontroverse Diskussionen, die gerne auch mal hitzig geführt werden dürfen. Beleidigende, grob anstößige, rassistische und strafrechtlich relevante Äußerungen und Beiträge tolerieren wir nicht. Bitte achte darauf, dass du keine Texte veröffentlichst, für die du keine ausdrückliche Erlaubnis des Urhebers hast. Ebenfalls nicht erlaubt ist der Missbrauch der Webangebote unter t3n.de als Werbeplattform. Die Nennung von Produktnamen, Herstellern, Dienstleistern und Websites ist nur dann zulässig, wenn damit nicht vorrangig der Zweck der Werbung verfolgt wird. Wir behalten uns vor, Beiträge, die diese Regeln verletzen, zu löschen und Accounts zeitweilig oder auf Dauer zu sperren.

Trotz all dieser notwendigen Regeln: Diskutiere kontrovers, sage anderen deine Meinung, trage mit weiterführenden Informationen zum Wissensaustausch bei, aber bleibe dabei fair und respektiere die Meinung anderer. Wir wünschen Dir viel Spaß mit den Webangeboten von t3n und freuen uns auf spannende Beiträge.

Dein t3n-Team

Schreib den ersten Kommentar!

Melde dich mit deinem t3n Account an oder fülle die unteren Felder aus.

Bitte schalte deinen Adblocker für t3n.de aus!

Hey du! Schön, dass du hier bist. 😊

Bitte schalte deinen Adblocker für t3n.de aus, um diesen Artikel zu lesen.

Wir sind ein unabhängiger Publisher mit einem Team bestehend aus 65 fantastischen Menschen, aber ohne riesigen Konzern im Rücken. Banner und ähnliche Werbemittel sind für unsere Finanzierung sehr wichtig.

Danke für deine Unterstützung.

Digitales High Five,
Stephan Dörner (Chefredakteur t3n.de) & das gesamte t3n-Team

Anleitung zur Deaktivierung