Ich habe mich schon sehr lange für Fragen zur Codeüberprüfung interessiert. Oft trat das eine oder andere Problem auf, entweder mit der Qualität des Codes oder mit dem Klima im Team. In der Tat ist die Codeüberprüfung, wenn nicht die einzige, einer der wichtigsten Orte für Konflikte im Entwicklungsteam.
Und vor kurzem habe ich in Vorbereitung auf die nächste Veröffentlichung des Zinc Prod- Podcasts herausgefunden, dass Google eine Richtlinie zur Codeüberprüfung veröffentlicht hat, die voller wertvoller Gedanken ist. Das gesamte Material ist ziemlich umfangreich und passt nicht in einen Artikel, daher werde ich versuchen, die interessantesten (für mich) Gedanken hervorzuheben.
Also lass uns gehen
Im Originalartikel verwendete Begriffe:
CL - Änderungsliste. Normalerweise wird es als Merge Request oder Pull Request bezeichnet. In diesem Artikel werde ich daher die Abkürzung MR verwenden
LGTM - Sieht gut aus für mich. Kurz gesagt, wenn sie die "Genehmigen" -Taste drücken. Ich werde den Begriff "aprov" verwenden, um ihn für die Bevölkerung verständlicher zu machen.
Idealität mr
In der Praxis kann man bei der Betrachtung von MR auf verschiedene Extreme stoßen. Jemand als ganzes Team zadalivaet kommentiert, bis alles auf den Punkt gebracht ist, korrigiert ist, jemand über die Logik schaut und "upruv" drückt.
Ein interessanter Gedanke ist im Google-Dokument geschrieben. MR sollte nicht perfekt sein, aber es sollte die Codebasis verbessern. Das heißt, mit jeder eingeführten Änderung sollte der Code immer besser werden. Und wenn MR viel Gutes hinzufügt, müssen Sie die kleinen Dinge nicht bemängeln. Es ist rentabler, diese Verbesserung schneller zu erzielen.
Kein MR sollte den Code verschlechtern. Die einzige Ausnahme ist, wenn MR eine dringende Lösung für etwas ist.
Freiheit zu kommentieren
Trotz der Tatsache, dass Sie Kleinigkeiten nicht loswerden können, können Sie diese Kleinigkeiten dennoch frei in mindestens jede Zeile schreiben. Der Widerspruch zum vorherigen Absatz wird einfach gelöst: Der Rezensent stellt Kleinigkeiten und Nit-Picking mit dem Präfix "nit:" aus dem Englischen voran. Nitpick (Nitpicking). Es ist nicht notwendig, solche Kommentare zu korrigieren. Der Autor von MR möchte jedoch möglicherweise etwas korrigieren oder, falls dies nicht der Fall ist, einige Punkte für die Zukunft berücksichtigen.
Fakten über persönliche Vorlieben
Fast immer sind die auf Best Practices basierenden Standardprinzipien für das Software-Design besser als knifflige Fahrräder. Daher sollten sie bevorzugt werden.
Wenn Sie mehrere Standardansätze anwenden können, liegt dies im Ermessen des Autors.
Direktes Zitat zum besseren Verständnis:
Aspekte des Software-Designs sind fast nie ein reines Stilproblem oder nur eine persönliche Präferenz. Sie basieren auf zugrunde liegenden Prinzipien und sollten auf diesen Prinzipien abgewogen werden, nicht nur durch persönliche Meinung. Manchmal gibt es einige gültige Optionen. Wenn der Autor nachweisen kann (entweder durch Daten oder basierend auf soliden technischen Prinzipien), dass mehrere Ansätze gleichermaßen gültig sind, sollte der Prüfer die Präferenz des Autors akzeptieren. Andernfalls wird die Auswahl durch Standardprinzipien des Software-Designs bestimmt.
Wenn der Gutachter und der Autor von MR nicht übereinstimmen
Zunächst wird versucht, in den Kommentaren zu MR einen Konsens zu erzielen. Wenn dies nicht funktioniert, dann eine persönliche Diskussion. Wenn Sie immer noch keinen Konsens erzielen, ziehen Sie Teammitglieder an. Vor allem aber sollte MR aufgrund der Meinungsverschiedenheit zweier Personen nicht lange stecken bleiben.
In Kommentaren besprochen -> Persönlich besprochen -> In einem Team besprochen -> Weitermachen
MR-Prüfgeschwindigkeit
Geschwindigkeit ist extrem wichtig. Wenn Sie alle paar Tage einen Kommentar abgeben, beschwert sich der Autor von MR, dass er einen Fehler findet und ihn am Arbeiten hindert.
Wenn MR sehr schnell überprüft wird, sind Kommentare kein großes Problem - schließlich werden ihre Korrekturen hier überprüft und die Aufgabe wird fortgesetzt.
Google rät Ihnen, sich nicht von Ihrer Aufgabe ablenken zu lassen. Wenn Sie jedoch abgelenkt sind, prüfen Sie, ob etwas zu überarbeiten ist. Zum Beispiel kehrte er vom Mittagessen zurück - er überarbeitete. Kam zur Arbeit - überarbeitet. Usw.
MR-Anzeigereihenfolge
Zuerst müssen Sie den MR als Ganzes betrachten. Ist es überhaupt notwendig, befindet sich der Code an der richtigen Stelle (oder sollte er sich in einer separaten Bibliothek befinden), gibt es globale Probleme?
Das heißt, Es macht keinen Sinn, einige Implementierungsdetails zu betrachten, wenn der Code nicht vorhanden ist und überhaupt nicht.
Im Allgemeinen ist die Anzeigereihenfolge wichtig, um dem Autor so schnell wie möglich ein Feedback zu geben (siehe vorherigen Absatz).
Wenn Sie MR als Ganzes betrachten, müssen Sie die Hauptdateien durchgehen, d. H. Überprüfen Sie das Wichtigste (wenn nicht klar ist, was am wichtigsten ist, können Sie den Entwickler fragen).
Auch hier sollten Probleme sofort gemeldet werden, auch wenn Sie den MR noch nicht inspiziert haben und beschlossen haben, ihn später allgemein zu inspizieren.
Als Nächstes müssen Sie eine logische Reihenfolge auswählen, um die verbleibenden Dateien anzuzeigen. Es sollte keine Datei übersprungen werden.
Worauf Sie beim Anzeigen achten müssen
- Der Code ist gut gestaltet
- Die Funktionalität ist aus Sicht der Benutzer dieses Codes gut gemacht, egal wer sie sind.
- Das Aussehen (falls vorhanden) sollte gut sein
- Alle Nuancen der parallelen Programmierung (falls vorhanden) werden berücksichtigt.
- Code nicht neu gestaltet
- Der Entwickler überarbeitet nicht: Sie müssen keinen Code schreiben, der möglicherweise benötigt wird oder nicht
- Der Code hat Tests
- Tests sind gut gestaltet
- Namen (für alles) sind gut ausgewählt
- Kommentare zum Code sind verständlich und notwendig. Sie müssen erklären, warum dies getan wird und nicht, wie es getan wird.
- Dokumentation hinzugefügt.
- Der Code entspricht Styleguides.
Zu großer MR
Zu große MRs müssen angefordert werden, um aufgelöst zu werden. Es ist fast immer möglich.
Wie schreibe ich Kommentare zur Codeüberprüfung?
- Sie müssen höflich sein, nicht persönlich werden. Diskutieren Sie den Code, nicht den Encoder.
- Geben Sie nicht nur Anweisungen für Korrekturen aus, sondern erklären Sie auch, warum Sie diese beheben müssen.
- Halten Sie ein Gleichgewicht aufrecht: Identifizieren Sie das Problem und fordern Sie den Entwickler auf, selbst zu verstehen, wie es am besten gelöst werden kann. oder geben Sie sofort eine fertige Lösung heraus. Der erste entwickelt den Entwickler (strategischer Nutzen), der zweite verbessert und beschleunigt den MR (taktischer Nutzen).
- Wenn der Prüfer irgendwann im Code nicht verstanden hat und den Autor auffordert zu erklären, was was ist, wäre die beste Antwort, den Code zu ändern. Damit war aus dem Code ohne Frage alles klar.
Wenn Sie Ihrer Meinung nicht zustimmen
Es ist notwendig, im Detail zu verstehen. Vielleicht verstehen Sie etwas nicht, fragen Sie nach weiteren Informationen. Vielleicht das Gegenteil. In jedem Fall kommt das Verständnis oft erst nach ein paar Erklärungsrunden auf beiden Seiten.
Angst, den Autor MR zu verärgern
Es kommt vor, dass der Entwickler verärgert ist, wenn der Prüfer auf einigen Änderungen besteht. Meistens sind die Entwickler jedoch wegen der Form des Kommentars und nicht wegen des Inhalts verärgert. Seien Sie höflich, erklären Sie mit Argumenten, und höchstwahrscheinlich wird alles in Ordnung sein.
"Ich werde es später beheben."
Wenn der Entwickler zustimmt, dass es ein Problem im Code gibt, aber nach MR fragt und verspricht, dass er es ein anderes Mal beheben wird, tritt dies nach Meinung der Jungs von Google meistens nie "später" auf. Wenn MR kein dringender Bugfix ist, müssen Sie daher auf einem Fix bestehen.
Schlussfolgerungen
Dieses Dokument von Google hat mir sehr gut gefallen. Besonders Life Hack mit dem Wort "nit", der Betonung der Geschwindigkeit der Codeüberprüfung und auch, dass MR nicht perfekt sein sollte. Es scheint einfach zu sein, aber solange dies eindeutig nicht berücksichtigt wird, erreicht es nicht den Punkt.
Wenn dieser Artikel für die Leser interessant erscheint und eine Reihe von Vorteilen aufgreift, schreibe ich den folgenden Teil: den Codeüberprüfungsprozess durch den Autor MR.
Update: Der zweite Teil des Artikels hier
In der kommenden Ausgabe von Zinc Sale werden wir den Überprüfungscode aus verschiedenen Blickwinkeln ausführlich diskutieren. Abonnieren Sie also unbedingt unseren Entwicklungs- Podcast !