Leitfaden für Code Reviews in Ihrem Team

Code Reviews sind eine Praxis, die sich in den letzten Jahren verbreitet hat, bei der ein oder mehrere Entwickler den neuen Code eines Kollegen überprüfen, mit dem Ziel, Qualitätsprobleme im Code, Bugs, Sicherheitslücken, schlechte Praktiken usw. zu erkennen. Dies ermöglicht es, Feedback-Schleifen zu verkürzen, was bekanntlich sehr vorteilhaft ist, denn je später ein Problem entdeckt wird, desto höher sind die Kosten für seine Behebung und desto größer ist die potenzielle geschäftliche Auswirkung.

Da Code das wesentliche Material ist, aus dem Software gebaut wird, und täglich von verschiedenen Entwicklern und Teams erstellt und gewartet wird, ist es entscheidend, Mechanismen zur Sicherstellung seiner Qualität zu haben. Gute Code Reviews durchzuführen, bewusst und nach Best Practices, kann den Unterschied zwischen Erfolg oder Misserfolg einer Anwendung und letztendlich eines Unternehmens ausmachen. Daher ist einer der ersten empfohlenen Schritte bei der Einführung von Code Reviews, einen Leitfaden zu haben, der dem gesamten Team Orientierung und Richtung gibt. Dieser Artikel soll als initialer Leitfaden dienen, den Teams nutzen und an ihren Kontext anpassen können.

Grundlagen von gutem Code

Bevor wir ins Detail gehen, finde ich es interessant, dieses Dokument zu teilen, das zwar Zen of Python heißt, aber ein guter Leitfaden für jede Sprache ist.

Das Schöne ist besser als das Hässliche.
Explizit ist besser als implizit.
Einfach ist besser als komplex.
Komplex ist besser als kompliziert.
Flach ist besser als verschachtelt.
Aufgelockert ist besser als dicht.
Lesbarkeit ist wichtig.
Sonderfälle sind nicht so besonders, dass sie die Regeln brechen dürfen.
Obwohl Praktikabilität vor Reinheit geht.
Fehler sollten niemals stillschweigend übergangen werden.
Es sei denn, sie werden ausdrücklich zum Schweigen gebracht.
Angesichts von Mehrdeutigkeit, widerstehen Sie der Versuchung zu raten.
Es sollte einen – und vorzugsweise nur einen – offensichtlichen Weg geben, es zu tun.
Obwohl dieser Weg anfangs nicht offensichtlich sein mag, es sei denn, Sie sind Niederländer.
Jetzt ist besser als nie.
Obwohl nie oft besser ist als *genau* jetzt.
Wenn die Implementierung schwer zu erklären ist, ist es eine schlechte Idee.
Wenn die Implementierung leicht zu erklären ist, könnte es eine gute Idee sein.
Namensräume sind eine großartige Idee – lasst uns mehr davon machen!

Quelle: https://peps.python.org/pep-0020/

Ziele von Code Reviews

(Stil) Abweichungen vom in den Stilrichtlinien definierten Stil erkennen (es ist ratsam, Stilrichtlinien für den Code unseres Unternehmens zu definieren) und im Code des Projekts.
(Architektur) Prüfen, ob die Implementierung der Architektur des Codes folgt.
(Modularität) Prüfen, ob der neue Code zu stark gekoppelt ist. Prüfen, ob die Komponenten Kohäsion haben, einen einzigen Zweck verfolgen.
(Komposition und Vererbung) Überprüfen, ob Vererbung und Komposition korrekt eingesetzt werden. Und ob der Komposition Vorrang vor der Vererbung gegeben wird.
(APIs) Wenn APIs erstellt oder erweitert werden, prüfen, ob sie konsistent und sauber sind.
(Lesbarkeit) Prüfen, ob der Code leicht zu verstehen, zu verfolgen und zu warten ist.
(Testbarkeit) Prüfen, ob der Code Unit-Tests und automatische Tests hat, die die neue Funktionalität testen.
(Fehlervermeidung) Mögliche Bugs und/oder vom Code nicht abgedeckte Fälle erkennen.
(Wissensaustausch) Die übrigen Entwickler mit den Änderungen im Code vertraut machen. Es ermöglicht, das Wissen besser im Team zu verteilen.
(Ausbildung) Wissenstransfer von erfahrenen Programmierern zu Neueinsteigern und Junioren.
(Sicherheit) Prüfen, ob der Code keine Anmeldedaten jeglicher Art preisgibt und keine offensichtlichen Sicherheitslücken aufweist.

Die Grundlage schaffen.

Das Ziel der Überprüfung ist nicht, die Arbeit der Kollegen zu kritisieren, sondern sie zu überprüfen, um sie gemeinsam zu verbessern, mögliche Versäumnisse oder Bugs zu erkennen und durch Feedback zu lernen. Kommentare sollten immer respektvoll sein und sollten offen aufgenommen werden, wobei wir unserem Kollegen für die Zeit danken, die er/sie mit der Überprüfung unseres Codes verbracht hat. Die Überprüfung sollte niemals als Wettbewerb zwischen Gegnern betrachtet werden, sondern als Teamarbeit.

Egal welche Seniorität wir als Entwickler haben, wir sind alle Menschen und wir alle machen Fehler. Ein guter Entwickler, egal wie erfahren, ist immer offen für Feedback, auch wenn es von einem weniger erfahrenen Entwickler kommt.

Vorgehensweise.

Vor der Überprüfung

Öffnen Sie den Pull Request/Merge Request (oder PR / MR) und lesen Sie die Beschreibung. Wenn Ihnen noch der Kontext für die Änderung fehlt, öffnen Sie das Ticket in Ihrer Projektmanagement-Software und lesen Sie es. Es ist wichtig, die Akzeptanzkriterien zu lesen, um zu versuchen zu erkennen, ob der Code alle erfüllen kann oder nicht **(natürlich ist der Zweck des Code Reviews nicht das Testen, aber in vielen Fällen können Sie nicht abgedeckte Akzeptanzkriterien allein durch Betrachten des Codes erkennen). **
Überprüfen Sie, ob der Zielbranch der richtige ist. Manchmal kann man versehentlich einen PR gegen main erstellen, wenn man ihn in einen anderen Branch integrieren wollte. Falls dies nicht der Fall ist, weisen Sie darauf hin und stoppen Sie die Überprüfung. Denn wenn der Zielbranch nicht der richtige ist, überprüfen wir möglicherweise bereits überprüfte Änderungen.

Überprüfung

Beginnen Sie mit der Überprüfung der Liste der geänderten/erstellten/gelöschten Dateien. Prüfen Sie, ob die Datei- und Modularchitektur mit den bestehenden Richtlinien und Best Practices (falls vorhanden) übereinstimmt. Prüfen Sie, ob die Namen der Namenskonvention folgen.
Wenn der Code Unit-Tests hat (empfohlen), kann es ein guter Einstieg sein, mit den Unit-Tests zu beginnen, um zu verstehen, ob alle Akzeptanzkriterien abgedeckt sind oder nicht. Es ist auch eine gute Möglichkeit, den Code von außen zu verstehen. Unit-Tests sollten nicht nur den Happy Path abdecken, sondern auch Grenzfälle und Fehlerfälle.
Prüfen Sie, ob der Stil des Codes dem Stilhandbuch folgt (falls vorhanden), wenn nicht, prüfen Sie, ob er einheitlich mit dem bestehenden Code ist. Prüfen Sie die Namen von Variablen, Klassen, Methoden usw.
Prüfen Sie, ob die Architektur des neuen Codes die bestehende Architektur respektiert. Zum Beispiel: Neue Klassen sind konsistent mit bestehenden Klassen. Die Art, wie der Code in Dateien organisiert ist, folgt den bestehenden Mustern.
Prüfen Sie, ob der Code lose gekoppelt ist und Kohäsion aufweist. Klassen und Methoden sollten einen einzigen Zweck haben. Ein häufiges Beispiel für starke Kopplung ist, wenn der Zugriff auf den Datenbankmotor für Abfragen über den gesamten Code verteilt ist. Dies führt zu einer starken Kopplung an den spezifischen Datenbankmotor und erschwert beispielsweise einen zukünftigen Austausch.
Prüfen Sie, ob kein Code dupliziert wurde. DRY (Don’t Repeat Yourself)
Prüfen Sie, ob der Code einfach zu verstehen ist. Wenn wir komplizierten Code sehen, sollten wir einen Kommentar zum PR hinzufügen, damit der Entwickler versucht, ihn zu vereinfachen und/oder Kommentare hinzuzufügen, damit er verstanden werden kann.
1. Prüfen Sie, ob keine magischen Zahlen oder Literale verwendet werden, was den Code weniger verständlich und weniger wartbar macht. Schlagen Sie die Verwendung von Konstanten mit einem selbsterklärenden Namen vor.
Wenn eine API geändert wurde, prüfen Sie, ob die Änderungen mit dem Rest der API konsistent sind, und prüfen Sie, ob die Dokumentation aktualisiert wurde.
Die Idee ist, den neuen Code zu überprüfen, aber manchmal müssen wir den Kontext, den bereits bestehenden Code, ein wenig lesen, um ihn gut zu verstehen. Manchmal kann man erkennen, dass die Änderung unvollständig oder nicht korrekt ist, wenn man sich den Kontext ansieht.
Prüfen Sie, ob der Code keine Passwörter, Anmeldedaten oder sensible Informationen jeglicher Art enthält. Wenn dies der Fall ist, befinden sich diese Informationen bereits im Repository und könnten kompromittiert sein, erwägen Sie eine Änderung der Anmeldedaten.
Achten Sie auf offensichtliche Sicherheitsprobleme, die mit bloßem Auge erkennbar sind. Zum Beispiel SQL-Injections (eine Benutzereingabe direkt in einer SQL-Abfrage verwenden).
Überprüfen Sie, ob die Änderungen sich in der Repository-Dokumentation (z.B. im README) widerspiegeln sollten, und prüfen Sie, ob sie aktualisiert wurde und korrekt verstanden wird. Zum Beispiel: Wenn die Art, das Projekt lokal zu starten, geändert wurde, sollte dies klar im README aktualisiert werden.

Nach der Überprüfung

Falls wir Kommentare abgegeben haben, achten Sie auf die mögliche Antwort und Änderungen im PR, um erneut zu überprüfen.
Wenn wir keine Kommentare abgegeben haben und es für gut befinden, kennzeichnen Sie dies im PR und akzeptieren Sie ihn. Hier kann es je nach den Regeln, die wir in unserem Team definiert haben, erforderlich sein, dass andere Personen ihn überprüfen und einem bestimmten Prozess folgen, um den PR zu mergen.

Abschließende Bemerkungen

Dieser Leitfaden dient als allgemeine Referenz für die einheitliche Durchführung von Code Reviews in einem Team/einer Organisation. Die Idee ist, dass jedes Team/jede Organisation ihn an seinen/ihren speziellen Fall anpassen und erweitern kann und kontinuierlich daran arbeitet, ihn zu verbessern und an die eigenen Umstände anzupassen.

Zusätzlich zu all diesen manuellen Prüfungen wird dringend empfohlen, statische Code-Analyse-Tools, Linter und Formatierungsregeln in der IDE, unter anderem, in den Prozess zu integrieren. Diese ermöglichen es uns, Probleme schnell und kostengünstig zu erkennen, noch bevor wir den PR erstellen, um die Code-Überprüfung anzufordern.

Und natürlich sind Ihre Kommentare immer willkommen.