Code Reviews bei FuPa

Published in

FuPa

4 min readMay 15, 2018

Unser größtes Ziel ist eine qualitativ hochwertige Plattform für den Amateurfußball zu bauen. Doch Qualität erfordert gute Planung, robuste Werkzeuge und nicht zuletzt einen durchdachten Prozess zur Qualitätssicherung. In der Entwicklungsabteilung setzen wir deshalb seit über einem Jahr intensiv auf Code Reviews!

Der Aufwand lohnt sich! 💪

Gute Code Reviews kosten einige Zeit. Dafür hat ein effektiver Review-Prozess aber auch zahlreiche Vorteile für die Software, die Plattform und das Entwicklungsteam. Die wichtigsten Vorteile für FuPa:

Effektive Prävention von Bugs
Entwickler sehen mehr Bereiche der Codebase
Validierung der Verständlichkeit des Codes
Erlernen und kommunizieren von Best Practices im Team
Erlernen eines professionellen Umgangs mit Git, Diffs & Merges

Vorbereitung ist Alles: Der Feature Branch

Jeder Entwickler beginnt neue Aufgaben in einem neuen Feature Branch. Dadurch sind die Änderungen dieser Aufgabe isoliert von anderen Aufgaben und insbesondere vom Code des Produktivsystems.

Isolation erlaubt uns maximale Unabhängigkeit und ist die Voraussetzung für unkomplizierte Code Reviews

Bei FuPa werden Feature Branches mit einem vorangestellten feature/ benannt. Danach folgt die Ticketnummer sowie eine kurze Beschreibung der Aufgabe. Beispiel: feature/12345-support-page

Entwicklung 👨‍💻👩‍💻

Während der Entwicklung an einer Aufgabe sollten folgende Aspekte beachtet werden:

A) Sauberen & einfach verständlichen Code schreiben

Vermeidung von Anti-Patterns und Nutzung bestehender Struktur.
Entwicklung von Clean Code (Bsp: Variablen mit beschreibenden Namen).

B) Das Rad nicht neu erfinden!

Es gibt npm/composer-Pakete für ziemlich alle Anwendungsfälle.
Speziell im Frontend: Anzahl & Größe der Abhängigkeiten minimal halten.

C) Nicht mehrere Aufgaben gleichzeitig bearbeiten

Jeder Feature Branch widmet sich exakt einem Problem.
Maximal wenige hundert geänderte Zeilen. Ansonsten ist eine effektive Code Review nahezu unmöglich.
Tipp: Jede Aufgabe lässt sich klar als 1) Feature, 2) Bugfix oder 3) Refactoring einordnen.

D) Den Regeln aus dem Styleguide folgen

Jedes Entwicklungsteam sollte so ein Dokument anlegen. Wenn sich alle dran halten, wird der Code besser strukturiert und deutlich lesbarer. Letzteres ist entscheidend für die Code Review!

E) Schlanke Commit Messages

Commit Messages sollten kurz, präzise und in imperativer Sprache verfasst sein.

Trommelwirbel: Der Merge Request

Sobald die Entwicklung abgeschlossen ist, wird auf Basis des Feature Branches ein Merge Request erstellt (aka “Pull Request” auf GitHub). Im Merge Request wird ein Teammitglied zur Review ausgewählt sowie der Zielbranch (bei uns develop).

An dieser Stelle ist — wie oftmals — die Dokumentation der Schlüssel zum Erfolg. Deshalb sollte eine Beschreibung der Änderungen oder zumindest ein Link zur Spezifikation der Aufgabe unbedingt in der Beschreibung für den Reviewer hinterlegt sein.

Code Review

Nun wirds ernst! Die eigentliche Code Review wird durchgeführt. Jetzt wird es tatsächlich zu einer Teamdisziplin! Bei der Code Review gibt es viele Aspekte zu beachten. Hier unsere Gedankenstütze für alle Reviewer:

Jede einzelne geänderte Zeile aktiv lesen und verstehen!
Struktur, Konsistenz, Bezeichnungen und Styleguide checken.
Keine Eile! Qualität(-schecks) benötigen Konzentration!
Bei unverständlichem Code: Kontakt zum Entwickler aufnehmen. Fragen stellen hilft beim Verständnis und deckt manchmal ungewolltes Verhalten auf.
Wichtig: Feature Branch auschecken und die Änderungen lokal nach testen.
Der Reviewer bittet den Autor um Änderung bzw. Anpassung. Der Reviewer sollte die Änderungen niemals selbst vornehmen. Nur damit findet ein schneller und effektiver Lernprozess statt!
Anmerkungen im Code dienen dazu Fragen zu stellen, Probleme aufzuzeigen und Verbesserungen vorzuschlagen. Wann immer möglich sollten zeilenbasierte Kommentare verwendet werden um unnötige Suchen zu ersparen.
Wenn alle Anmerkungen gemacht sind, wird der Merge Request wieder zurück an den Autor zugewiesen. In diesem Fall springt der Prozess zurück zu “Entwicklung”. Wenn es keine Anmerkungen gibt wird der Merge durchgeführt.

Der Ton macht die Musik 🎵

Ganz wichtig: Niemanden persönlich angreifen! Konstruktives Feedback bittet höflich darum den Code anzupassen. 🙂
Wenn man als Reviewer auf besonders ausgetüftelte Lösungen trifft: Lob unbedingt an den Autor ausrichten — gerne etwas öffentlich (z.B. im Slack) 💙

Deployment 🚀

Als letzten Schritt kümmern wir uns darum unsere Änderungen ins Produktivsystem zu deployen. Das geschieht ganz einfach über einen git merge von develop in master und anschließenden Push. Unsere GitLab Runner kümmern sich um den Rest wie Build erstellen, Tests durchführen und schließlich das Deployment aufs Produktivsystem. 🤖

Teamwork! 🙌

Der Erfolg der Code Reviews ist vor allem erfolgreiches Teamwork, einer der Core Values der FuPa GmbH. Gegenseitiges Verständnis, intensive Zusammenarbeit und regelmäßiges Feedback sind dabei entscheidend.