Zweck von Code Reviews
Qualitätssicherung
Frühzeitiger Fehlererkennung
Wissensverbreitung (im Team)
für Reviewer
konstruktiv und respektvoll bleiben
(kleine) fokussierte Reviews bevorzugen
Review mit Kontext (Anforderungen, Ziel) durchführen
eigene Annahmen hinterfragen
auf Verständlichkeit und Stil achten (Semantik von Bezeichnern etc.)
für Autoren
Reviews nicht persönlich nehmen
Nachfragen ggf. als Hinweis verstehen für Dokumentationsmängel
der Code ist konsistent formatiert (Code Prettifier)
triviale Fehler wurden bereits behoben (Linting)
der Code kompiliert
(Unit-/Integrations-) Tests laufen durch
Architektur und zentrale Designentscheidungen sind dokumentiert
Exemplarische Kriterien
keine Redundanzen, keine unnötigen Style Definitionen.
Konsistente Schreibweise von ....
CSS Variablen
Klassennamen
IDs
Sind die Namen (CSS Variablen, Klassennamen und IDs) verständlich und sprechend?
(Korrektes Deutsch oder Englisch!)
Werden CSS Variablen sinnvoll verwendet? (Z.B. um ein leicht anpassbares Design zu ermöglichen?)
Werden CSS Layer verwendet, um die Styles zu organisieren und die Wartbarkeit zu erhöhen (Stichwort: Modularisierung)?
Wird CSS Nesting verwendet, um die Struktur der Styles zu verdeutlichen?
Einfachheit und Korrektheit von Selektoren
(Z. B. keine Verwendung von !important.)
Sind die Selektoren effizient?
Sind komplizierte Selektoren - falls notwendig - dokumentiert/begründet/nachhvollziehbar?
Konsistente Vorgehensweise in Hinblick auf (z. B.) Responsiveness (z. B. konsistente Verwendung von Größen (rem, rem,...) oder das Farbschema.
Einfachheit des Layouts?
Gibt es ein klares Vorgehen (Mobile-first or Desktop-first)?
Werden Tools eingesetzt, um bei der Formatierung bzw. dem Linting zu helfen.
Wird modernes CSS verwendet?
(Dies gilt immer in Hinblick auf die Zielplattformen/Browserversionen.)
Exemplarische Kriterien
Ist der HTML Code korrekt?
Benennung von Klassen und IDs:
Sind die Namen konsistent vergeben und verständlich und korrekt geschrieben.
Ist die Struktur einfach nachvollziehbar oder übermäßig komplex?
(Z. B. gibt es eine hohe Anzahl and verschachtelten DIVs oder gibt es eine sinnvolle Strukturierung der Überschriften?).
Accessibility?
Nutzt der Code nur <span> und <div>, oder semantisches HTML?
Werden ARIA Tags sinnvoll verwendet?
Haben Bilder ein alt Attribut?
Keine Verwendung von inlines styles?
Keine Verwendung von inline JavaScript?
Keine Verwendung von <br> für Formatierungszwecke.
Konsistente Verwendung von tags?
(Z. B. ist Code immer in <pre><code>...</code></pre>?)
Werden Eingaben in Eingabefelder geprüft?
Werden meta Tags für die Spezifikation des Viewports sowie weiterer Metainformationen verwendet?
Werden übermäßig große Bilder/Icons vermieden?
Sind Links inhaltlich nachvollziehbar?
Exemplarische Kriterien
Macht der Code was er vorgibt zu tun?
(D. h. sind Variablen, Klassen, Methoden, ... -namen korrekt und sinnvoll?)
Werden Sonderfälle und Fehlerzustände behandelt?
Ist die Fehlerbehandlung konsistent und für den Endnutzer nachvollziehbar?
Werden keine Fehler „verschluckt“?
Wird null/undefined korrekt behandelt?
Wird modernes JavaScript verwendet (z. B. Klassen, const und let anstatt von var, Destrukturierung, Spread und Rest Operator etc.)?
Wird eval() nicht verwendet?
Werden asynchrone Funktionen richtige verwendet?
Ist der Kontrollfluss verständlich?
(Nachfragen deuten auf Probleme bzgl. der Verständlichkeit hin.)
Ist der Code modularisiert?
Wird auf tief verschachtelte Logik verzichtet?
Werden teure Manipulationen (des DOMs) auf das notwendige Minimum beschränkt?
Werden Eingaben validiert (auf Client und Server Seite)?
Ist das Logging von Fehlern (Error) sinnvoll und enthält genug Kontextinformationen?
Gibt es Testfälle? Falls ja, sind diese ausreichend?
(Codeabdeckung ist hier nur ein erster Indikator.)
Wird nur minimaler globaler Zustand verwendet?
Ist die (Inline-)Dokumentation ausreichend und korrekt.
Sind die TODOs, FIXMEs verständlich und umsetzbar?
Bei verteilten Anwendungen: ist die Aufteilung der Logik nachvollziehbar und sinnvoll (zum Beispiel auch aus Sicht von Cheatingmöglichkeiten)