Aus der Dokumentation zu den technischen Praktiken von Google

Dieser Leitfaden enthält Best Practices für die Durchführung von Codeüberprüfungen auf der Grundlage jahrelanger Erfahrung. Zusammen bilden sie ein Dokument, das in viele Abschnitte unterteilt ist. Es ist nicht notwendig, alle zu lesen, aber oft ist es für Sie und Ihr Team besser, den Leitfaden vollständig zu lesen.

• Code Review Standard
  
• Was Code einchecken
  
• Listennavigation ändern (CL)
  
• Geschwindigkeit der Codeüberprüfung
  
• Wie schreibe ich Kommentare?
  
• Wie man Widerstand überwindet

Ausführliche Hinweise zu Entwicklern, deren Commits überprüft werden, finden Sie auch im CL-Autorenhandbuch .

Code Review Standard

Der Hauptzweck einer Codeüberprüfung besteht darin, eine kontinuierliche Verbesserung der Google-Codebasis zu gewährleisten. Alle Tools und Prozesse sind diesem Ziel gewidmet.

Hier sind einige Kompromisse erforderlich.

Erstens sollten Entwickler in der Lage sein , ihre Probleme erfolgreich zu lösen . Wenn Sie niemals einen Code senden, wird sich die Codebasis niemals verbessern. Wenn der Prüfer die Arbeit erheblich erschwert, sind Entwickler in Zukunft nicht daran interessiert, Verbesserungen vorzuschlagen.

Andererseits liegt es in der Verantwortung des Prüfers, sicherzustellen, dass die Qualität des CL die Gesamtqualität der Codebasis im Laufe der Zeit nicht beeinträchtigt. Dies kann schwierig sein, da häufig eine Verschlechterung aufgrund einer leichten Verschlechterung der Qualität des Codes im Laufe der Zeit auftritt, insbesondere wenn das Team unter starkem Druck der Fristen steht und der Ansicht ist, dass es das Recht hat, die technische Verschuldung zu erhöhen.

Darüber hinaus ist der Prüfer für den zu prüfenden Code verantwortlich. Er möchte sicherstellen, dass die Codebasis konsistent bleibt, unterstützt wird und mit allem übereinstimmt, was im Abschnitt „Was muss Code eingecheckt werden?“ Erwähnt wird.

Daher erhalten wir die folgende Regel als Standard für die Codeüberprüfung:

In der Regel sollten Prüfer den CL unterstützen, sobald er einen Zustand erreicht, in dem die Gesamtqualität des Systemcodes definitiv verbessert wird, auch wenn der CL nicht perfekt ist.

Dies ist die Hauptüberprüfung des Codes unter allen Prinzipien.

Natürlich hat er Einschränkungen. Wenn CL beispielsweise eine Funktion hinzufügt, die der Prüfer nicht im System sehen möchte, kann der Prüfer das Festschreiben mit Sicherheit ablehnen, selbst wenn der Code von guter Qualität ist.

Der entscheidende Punkt hierbei ist, dass es keinen „perfekten“ Code gibt - es gibt nur besseren Code. Der Rezensent sollte nicht verlangen, dass der Autor jedes winzige Fragment poliert. Der Prüfer sollte vielmehr die Notwendigkeit weiterer Fortschritte in Bezug auf die Bedeutung der vorgeschlagenen Änderungen ausgleichen. Anstatt nach dem Ideal zu streben, sollte der Rezensent nach kontinuierlicher Verbesserung streben. Ein Commit, das im Allgemeinen die Wartbarkeit, Lesbarkeit und Verständlichkeit des Systems verbessert, kann nicht um Tage oder Wochen verzögert werden, da es nicht "perfekt" ist.

Prüfer können jederzeit Kommentare zur Verbesserung des Codes hinterlassen, aber nicht sehr wichtige Änderungen sollten beispielsweise mit dem Nit- Präfix gekennzeichnet werden, damit der Autor weiß, dass dies nur ein Gesichtspunkt ist, den er ignorieren kann.

Hinweis Nichts in diesem Dokument rechtfertigt CLs, die die Gesamtqualität des Systemcodes definitiv beeinträchtigen . Dies ist nur im Notfall möglich .

Mentoring

Eine Codeüberprüfung kann auch wichtig sein, um Entwicklern etwas Neues über die Sprache, Struktur oder allgemeinen Prinzipien des Software-Designs beizubringen. Es ist immer schön, Kommentare zu hinterlassen, die dem Entwickler helfen, etwas Neues zu lernen. Der Austausch von Wissen trägt zur Verbesserung des Systemcodes im Laufe der Zeit bei. Denken Sie daran, dass Sie, wenn Sie einen rein pädagogischen Kommentar hinterlassen, der für die Erfüllung der hier beschriebenen Standards nicht kritisch ist, das Nit- Präfix hinzufügen : oder auf andere Weise angeben, dass der Autor nicht verpflichtet ist, dies zuzulassen.

Prinzipien

• Technische Fakten und Daten überwiegen Meinungen und persönliche Vorlieben.
  
• In Sachen Stil ist absolute Autorität der Leitfaden für Stil . Jedes rein stilistische Detail (Raum usw.), das nicht im Styleguide enthalten ist, ist eine Frage der persönlichen Präferenz. Der Stil sollte dem entsprechen, was er ist. Wenn es keinen vorherigen Stil gibt, akzeptieren Sie den Autor.
  
• Aspekte des Software-Designs sind fast nie eine Frage des reinen Stils oder der persönlichen Vorlieben. Sie basieren auf Grundprinzipien und sollten von diesen Prinzipien bestimmt werden und nicht nur von der persönlichen Meinung. Manchmal gibt es mehrere gültige Optionen. Wenn der Autor nachweisen kann (entweder anhand von Daten oder basierend auf soliden technischen Prinzipien), dass bestimmte Ansätze gleichermaßen effektiv sind, sollte der Prüfer die Präferenz des Autors akzeptieren. Andernfalls wird die Auswahl durch Standardentwicklungsprinzipien bestimmt.
  
• Wenn keine andere Regel anwendbar ist, kann der Prüfer den Autor auffordern, die Einheitlichkeit mit der aktuellen Codebasis zu beachten, wenn dies den allgemeinen Zustand des Systems nicht verschlechtert.

Konfliktlösung

In jedem Konflikt sollte der erste Schritt immer der Wunsch des Entwicklers und Überprüfers sein, auf der Grundlage des Inhalts dieses Dokuments und anderer Dokumente im CL Author's Guide und im Reviewer Guide zu einem Konsens zu gelangen.

Wenn es besonders schwierig ist, einen Konsens zu erzielen, kann ein persönliches Treffen oder eine Videokonferenz zwischen dem Rezensenten und dem Autor hilfreich sein (wenn Sie dies tun, schreiben Sie die Ergebnisse der Diskussion in einem Kommentar zum Commit für zukünftige Leser auf).

Wenn dies die Situation nicht löst, ist der häufigste Weg die Eskalation. Oft besteht es in einer breiteren Diskussion mit dem Team, der Gewinnung von Teamleitern und der Kontaktaufnahme mit dem Betreuer oder Entwicklungsmanager. Lassen Sie das Commit nicht verweilen, da der Autor und der Rezensent keine Einigung erzielen können.

Was Code einchecken

Hinweis Beachten Sie bei der Betrachtung der einzelnen Punkte unbedingt die Überprüfung des Standardcodes .

Design

Das Wichtigste ist, das Gesamtprojekt (Design) bei der Codeüberprüfung zu berücksichtigen. Sind Interaktionen zwischen verschiedenen Teilen des Codes sinnvoll? Gilt diese Änderung für Ihre Codebasis oder Bibliothek? Integriert sich CL gut in den Rest des Systems? Ist es Zeit, diese Funktionalität jetzt hinzuzufügen?

Funktionalität

Tut dieser CL das, was der Entwickler beabsichtigt hat? Ist es gut für Benutzer dieses Codes? Mit "Benutzer" meinen wir sowohl Endbenutzer (wenn sie von der Änderung betroffen sind) als auch Entwickler (die diesen Code in Zukunft "verwenden" müssen).

Grundsätzlich erwarten wir, dass Entwickler ihren Code bereits vor dem Festschreiben gut genug testen, damit er ordnungsgemäß funktioniert. Als Prüfer müssen Sie jedoch immer noch über Extremfälle nachdenken, nach Parallelitätsproblemen suchen, versuchen, als Benutzer zu denken, und sogar beim Lesen des Codes beobachten, dass keine offensichtlichen Fehler vorliegen.

Wenn Sie möchten, können Sie die Leistung überprüfen. Dies ist am wichtigsten, wenn der Code Benutzer betrifft, z. B. das Ändern der Benutzeroberfläche . Es ist schwer zu verstehen, wie sich einige Änderungen auf Benutzer auswirken, wenn Sie nur den Code lesen. Für solche Änderungen können Sie den Entwickler bitten, eine Demo bereitzustellen, wenn es für Sie zu schwierig ist, sich mit dem Code zu befassen und ihn selbst auszuprobieren.

Ein weiterer Punkt, an dem es besonders wichtig ist, während einer Codeüberprüfung über die Funktionalität nachzudenken, ist, wenn im CL eine Art parallele Programmierung auftritt, die theoretisch zu Deadlocks oder Race-Bedingungen führen kann. Solche Probleme sind sehr schwer zu erkennen, wenn Sie einfach den Code ausführen. In der Regel muss jemand (sowohl der Entwickler als auch der Prüfer) sorgfältig darüber nachdenken und sicherstellen, dass keine Probleme auftreten (beachten Sie, dass dies auch ein guter Grund ist, keine Parallelitätsmodelle zu verwenden, bei denen Rennbedingungen oder Deadlocks möglich sind - dies kann per Code erfolgen sehr schwer zu verstehen oder Codeüberprüfung).

Schwierigkeit

Ist CL komplizierter als es sein sollte? Überprüfen Sie es auf jeder Ebene: separate Zeilen, Funktionen, Klassen. "Übermäßige Komplexität" bedeutet normalerweise die Unfähigkeit, beim Lesen schnell zu verstehen . Dies kann auch bedeuten, dass Entwickler beim Versuch, diesen Code aufzurufen oder zu ändern , mit größerer Wahrscheinlichkeit Fehler einführen .

Eine besondere Art von Komplexität ist das Über-Engineering, wenn Entwickler den Code universeller gemacht haben, als er sein sollte, oder Funktionen hinzugefügt haben, die das System derzeit nicht benötigt. Gutachter sollten besonders wachsam sein, wenn es um Überentwicklung geht. Ermutigen Sie die Entwickler, ein Problem zu lösen, das jetzt definitiv gelöst werden sollte, anstatt ein Problem, das möglicherweise in Zukunft gelöst werden muss. Ein zukünftiges Problem sollte gelöst werden, wenn es auftritt, und Sie können seine tatsächliche Form und Anforderungen im physischen Universum sehen.

Tests

Fordern Sie Unit-, Integrations- oder End-to-End-Tests an, die für die Änderung relevant sind. Im Allgemeinen sollten Tests zu demselben CL wie der Produktionscode hinzugefügt werden, wenn der CL den Notfall nicht behandelt.

Stellen Sie sicher, dass die Tests korrekt, angemessen und hilfreich sind. Tests testen sich nicht selbst und wir schreiben selten Tests für unsere Tests - eine Person muss sicherstellen, dass die Tests gültig sind.

Scheitern Tests bei defektem Code wirklich? Wenn sich der Code ändert, gibt es dann falsch positive Ergebnisse? Macht jeder Test einfache und nützliche Aussagen? Sind die Tests korrekt auf verschiedene Testmethoden aufgeteilt?

Denken Sie daran, dass Tests Code sind, der ebenfalls unterstützt werden muss. Lassen Sie keine Komplexität zu, nur weil sie nicht Teil der Hauptbinärdatei ist.

Benennen

Hat der Entwickler überall gute Namen gewählt? Ein guter Name ist lang genug, um vollständig zu vermitteln, was das Element ist oder was es tut, ohne so lang zu sein, dass es schwer zu lesen ist.

Kommentare

Hat der Entwickler klare Kommentare im Klartext geschrieben? Sind alle Kommentare notwendig? Kommentare sind normalerweise nützlich, wenn sie erklären, warum Code vorhanden ist, und nicht erklären sollten, was dieser Code bewirkt. Wenn der Code nicht klar genug ist, um sich selbst zu erklären, sollte er vereinfacht werden. Es gibt einige Ausnahmen (zum Beispiel sind Kommentare, die die Aktionen des Codes erläutern, oft sehr nützlich für reguläre Ausdrücke und komplexe Algorithmen), aber meistens sind Kommentare für Informationen gedacht, die der Code selbst nicht enthalten kann, zum Beispiel um eine Entscheidung zu begründen.

Es kann auch nützlich sein, die Kommentare im vorherigen Code zu lesen. Möglicherweise gibt es ein TODO, das jetzt gelöscht werden kann, oder einen Kommentar, der die Einführung dieser Änderung usw. nicht empfiehlt.

Beachten Sie, dass sich Kommentare von der Dokumentation für Klassen, Module oder Funktionen unterscheiden, in der die Aufgabe des Codes, seine Verwendung und sein Verhalten beschrieben werden.

Stil

Wir haben Google Style Guides für alle wichtigen Sprachen und sogar für die meisten kleineren Sprachen. Stellen Sie sicher, dass der CL nicht mit den entsprechenden Styleguides in Konflikt steht.

Wenn Sie einige Elemente verbessern möchten, die nicht im Styleguide enthalten sind, fügen Sie dem Kommentar eine Notiz hinzu ( Nit :) . Der Entwickler wird wissen, dass dies Ihre persönliche Bemerkung ist, die nicht bindend ist. Blockieren Sie das Senden von Commits nicht ausschließlich aufgrund Ihrer persönlichen Vorlieben.

Der Autor sollte keine wesentlichen Stiländerungen mit anderen Änderungen kombinieren. Dies macht es schwierig, Änderungen im CL zu erkennen, erschwert Zusammenführungen, Code-Rollbacks und verursacht andere Probleme. Wenn der Autor beispielsweise die gesamte Datei neu formatieren möchte, fordern Sie eine Stiländerung in einem CL an und senden Sie den CL mit Funktionsänderungen.

Die Dokumentation

Wenn die CL-Ausgabe Änderungen an der Assembly, den Tests, den Interaktionsverfahren oder der Codefreigabe ändert, suchen Sie nach Aktualisierungen der entsprechenden Dokumentation, einschließlich README-Dateien, g3doc-Seiten und aller generierten Referenzdokumente. Wenn der CL Code entfernt oder veraltet macht, überlegen Sie, ob Sie auch die Dokumentation entfernen möchten. Wenn die Dokumentation fehlt, fordern Sie ihre Erstellung an.

Jede Reihe

Zeigen Sie jede Zeile im Code an. Obwohl Datendateien, generierter Code oder große Datenstrukturen kurz überprüft werden können, aber jede von einer Person geschriebene Klasse, Funktion oder jeden Codeblock sorgfältig lesen, sollten Sie niemals standardmäßig davon ausgehen, dass alles in Ordnung ist. Natürlich verdient ein Code eine gründlichere Untersuchung als ein anderer - Sie entscheiden selbst, aber Sie sollten zumindest sicher sein, dass Sie die Funktionsweise des gesamten Codes verstehen .

Wenn es Ihnen zu schwer fällt, den Code zu lesen, und dies die Überprüfung verlangsamt, sollten Sie den Entwickler informieren und warten, bis er Klarheit schafft, bevor Sie fortfahren. Bei Google stellen wir großartige Software-Ingenieure ein, und Sie sind einer von ihnen. Wenn Sie den Code nicht verstehen können, ist es sehr wahrscheinlich, dass andere Entwickler dies nicht können. Auf diese Weise helfen Sie auch zukünftigen Entwicklern, diesen Code zu verstehen, wenn Sie den Entwickler um Klarheit bitten.

Wenn der Code verständlich ist, Sie sich jedoch nicht qualifiziert genug fühlen, um ein bestimmtes Fragment zu bewerten, stellen Sie sicher, dass sich im CL ein Prüfer befindet, der qualifiziert ist, insbesondere für komplexe Probleme wie Sicherheit, Parallelität, Zugänglichkeit, Internationalisierung usw.

Kontext

Es ist oft nützlich, CL in einem breiten Kontext zu betrachten. In der Regel zeigt ein Codeüberprüfungstool nur wenige Zeilen in der Nähe des Änderungsorts an. Manchmal müssen Sie sich die gesamte Datei ansehen, um sicherzustellen, dass die Änderung wirklich sinnvoll ist. Sie sehen beispielsweise nur vier Zeilen, aber wenn Sie sich die gesamte Datei ansehen, befinden sich diese vier Zeilen in der 50-Zeilen-Methode, die jetzt wirklich in kleinere Zeilen unterteilt werden muss.

Es ist auch nützlich, über CL im Kontext des Gesamtsystems nachzudenken. Verbessert es die Qualität des Systemcodes oder macht es komplexer, weniger getestet usw.? Akzeptieren Sie keine Commits, die den Systemcode verschlechtern. Die meisten Systeme werden durch die Summe vieler kleiner Änderungen kompliziert. Daher ist es wichtig, auch kleinere Schwierigkeiten zu vermeiden.

Gut

Wenn Sie im Commit etwas Gutes sehen, teilen Sie dies dem Entwickler mit, insbesondere wenn er das in einem Ihrer Kommentare beschriebene Problem bestmöglich gelöst hat. Codeüberprüfungen konzentrieren sich oft nur auf Fehler, sollten aber auch bewährte Verfahren fördern und bewerten. Unter dem Gesichtspunkt des Mentorings ist es manchmal noch wichtiger, dem Entwickler zu sagen, was genau er richtig gemacht hat und nicht, wo er einen Fehler gemacht hat.

Zusammenfassung

Stellen Sie beim Ausführen einer Codeüberprüfung Folgendes sicher:

• Der Code ist gut gestaltet.
  
• Die Funktionalität ist gut für Codebenutzer.
  
• Alle Änderungen an der Benutzeroberfläche sind angemessen und sehen gut aus.
  
• Jede gleichzeitige Programmierung ist sicher.
  
• Der Code ist nicht komplizierter als er sein sollte.
  
• Der Entwickler implementiert nicht, was in Zukunft mit unbekannten Perspektiven benötigt wird.
  
• Der Code ist mit entsprechenden Unit-Tests versehen.
  
• Die Tests sind gut gestaltet.
  
• Der Entwickler verwendete überall eindeutige Namen.
  
• Kommentare sind verständlich und nützlich und beantworten im Grunde die Frage warum? aber nicht was?
  
• Der Code ist ordnungsgemäß dokumentiert (normalerweise in g3doc).
  
• Der Code entspricht unseren Styleguides.

Achten Sie während der Überprüfung darauf, jede Codezeile zu betrachten, den Kontext zu betrachten , sicherzustellen, dass Sie die Qualität des Codes verbessern, und loben Sie die Entwickler für das Gute , das sie geschafft haben.

CL-Navigation in der Codeüberprüfung

Zusammenfassung

Nachdem Sie nun wissen, was Sie Code einchecken müssen , wie können Sie Codeüberprüfungen für mehrere Dateien am effizientesten durchführen?

1. Ist diese Änderung sinnvoll? Hat er eine gute Beschreibung ?
   
2. Schauen Sie sich zuerst den wichtigsten Teil an. Ist es gut gestaltet?
   
3. Schauen Sie sich den Rest des CL in der entsprechenden Reihenfolge an.

Schritt eins: Schauen Sie sich das gesamte Commit an

Schauen Sie sich die CL-Beschreibung an und was sie im Allgemeinen macht. Ist diese Änderung überhaupt sinnvoll? Wenn es ursprünglich nicht hätte geschrieben werden sollen, antworten Sie bitte sofort mit einer Erklärung, warum es nicht benötigt wird. Wenn Sie eine solche Änderung ablehnen, ist es auch hilfreich, dem Entwickler anzubieten, was stattdessen zu tun ist.

Zum Beispiel könnten Sie sagen: "Sieht so aus, als hätten Sie gute Arbeit geleistet, danke!" Wir werden das FooWidget-System jedoch tatsächlich entfernen, sodass wir derzeit keine neuen Änderungen vornehmen möchten. Wie wäre es, wenn Sie stattdessen unsere neue BarWidget-Klasse umgestalten? “

Bitte beachten Sie, dass der Prüfer den aktuellen CL nicht nur abgelehnt und einen alternativen Vorschlag gemacht hat, sondern dies auch höflich getan hat. Diese Höflichkeit ist wichtig, weil wir zeigen wollen, dass wir uns als Entwickler respektieren, auch wenn wir uns nicht einig sind.

Wenn Sie einige unerwünschte CLs erhalten, sollten Sie den Entwicklungsprozess in Ihrem Team überprüfen oder Regeln für externe Mitarbeiter veröffentlichen, um die Kommunikation beim Schreiben des CL zu verbessern. Es ist besser, den Leuten "Nein" zu sagen, bevor sie eine Menge Arbeit erledigen, die weggeworfen oder schwer umgeschrieben werden muss.

Schritt zwei: Lernen Sie die grundlegenden Teile von CL

Suchen Sie die Datei oder Dateien, die den "Hauptteil" dieses CL darstellen. Oft gibt es eine Datei mit den logischsten Änderungen, und dies ist der Hauptteil des CL. Schauen Sie sich zuerst diese Hauptteile an. Dies hilft, den Kontext kleinerer Teile des CL zu verstehen, und beschleunigt im Allgemeinen die Ausführung der Codeüberprüfung. Wenn der CL zu groß ist, fragen Sie den Entwickler, was er zuerst sehen soll, oder bitten Sie ihn, den CL in Teile aufzuteilen .

Wenn im Hauptteil des CL schwerwiegende Probleme auftreten, sollten Sie diese Kommentare sofort senden, auch wenn Sie momentan keine Zeit haben, den Rest des CL anzuzeigen. In der Tat kann es Zeitverschwendung sein, den Rest des Codes zu betrachten, denn wenn die Probleme erheblich genug sind, verschwinden viele der anderen fraglichen Codeteile und spielen keine Rolle.

Es gibt zwei Hauptgründe, warum es so wichtig ist, diese Hauptkommentare sofort zu senden:

• Entwickler senden häufig CL und starten dann sofort einen neuen Job basierend darauf, während sie auf das Ergebnis einer Codeüberprüfung warten. Wenn der CL ernsthafte Probleme hat, muss er den nächsten CL überarbeiten. Es ist ratsam, Fehler so früh wie möglich zu erkennen, bevor sie zusätzlich zum problematischen Design viel zusätzliche Arbeit leisten.
  
• Wichtige Änderungen dauern länger als kleinere Änderungen.Fast alle Entwickler haben Fristen. Um in ihnen zu bleiben und die Qualität des Codes nicht zu beeinträchtigen, sollte jedes größere Refactoring so bald wie möglich beginnen.

Schritt 3: Durchsuchen Sie den Rest des CL in der entsprechenden Reihenfolge.

Nachdem Sie sichergestellt haben, dass beim Entwurf des CL insgesamt keine ernsthaften Probleme auftreten, versuchen Sie, die logische Reihenfolge für die Anzeige von Dateien zu ermitteln und sicherzustellen, dass nichts übersehen wird. Wenn Sie sich die Hauptdateien ansehen, ist es normalerweise am einfachsten, den Rest in der Reihenfolge durchzugehen, in der sie vom Codeüberprüfungstool angezeigt werden. Manchmal ist es auch nützlich, zuerst die Tests und dann den Hauptcode zu lesen, da Sie dann eine Vorstellung davon haben, was die Änderung bedeutet.

Geschwindigkeit der Codeüberprüfung

Warum sollte eine Codeüberprüfung schnell durchgeführt werden?

Bei Google optimieren wir die Geschwindigkeit der Zusammenarbeit , nicht die Geschwindigkeit einzelner Entwickler. Die Geschwindigkeit der individuellen Arbeit ist wichtig, sie hat im Vergleich zur Geschwindigkeit der Teamarbeit einfach keine so hohe Priorität.

Wenn Sie sich den Code langsam ansehen, passieren verschiedene Dinge:

• Die Geschwindigkeit des gesamten Teams nimmt ab. Ja, wenn eine Person nicht schnell auf eine Codeüberprüfung reagiert, erledigt sie einen anderen Job. Für den Rest des Teams verzögern sich neue Funktionen und Fehlerbehebungen jedoch um Tage, Wochen oder Monate, da jeder CL auf eine Codeüberprüfung und eine wiederholte Codeüberprüfung wartet.
  
• -. , , . , «» . (, ), , . - .
  
• Die Qualität des Codes kann darunter leiden. Wenn Sie die Codeüberprüfung verlangsamen, steigt das Risiko, dass Entwickler nicht so gute CLs senden, wie sie könnten. Langsame Überprüfungen behindern auch die Bereinigung, das Refactoring und weitere Verbesserungen bestehender CLs.

Wie führe ich schnell eine Codeüberprüfung durch?

Wenn Sie sich nicht mitten in einer fokussierten Aufgabe befinden, sollten Sie kurz nach dem Eintreffen einen Überprüfungscode erstellen.

Ein Werktag ist die maximale Zeit für eine Antwort (das heißt, dies ist das erste, was am nächsten Morgen geschieht).

Das Befolgen dieser Richtlinien bedeutet, dass ein typischer CL innerhalb eines Tages mehrere Überprüfungsrunden (falls erforderlich) erhalten sollte.

Geschwindigkeit und Ablenkung

Es gibt einen Moment, in dem die Priorität der persönlichen Geschwindigkeit die Teamgeschwindigkeit übersteigt. Wenn Sie sich mitten in einer gezielten Aufgabe befinden, z. B. beim Schreiben von Code, lassen Sie sich nicht von der Codeüberprüfung ablenken. Studien haben gezeigt, dass es lange dauern kann, bis ein Entwickler nach der Ablenkung wieder zu einem reibungslosen Entwicklungsfluss zurückkehrt. Die Ablenkung vom Codieren kostet das Team also mehr, als einen anderen Entwickler zu bitten, ein wenig auf die Codeüberprüfung zu warten.

Warten Sie stattdessen auf einen Haltepunkt in Ihrer Arbeit. Zum Beispiel nach Abschluss der aktuellen Aufgabe, nach dem Mittagessen, nach der Rückkehr von einer Besprechung, nach der Rückkehr aus einer Büroküchenzeile usw.

Schnelle Antworten

Wenn wir über die Geschwindigkeit der Codeüberprüfung sprechen, interessiert uns die Antwortzeit und nicht, wie viel Zeit für den gesamten Prozess bis zum Ende benötigt wird. Idealerweise sollte der gesamte Prozess auch schnell sein, aber es ist noch wichtiger, dass einzelne Antworten schnell eintreffen als der gesamte Prozess .

Auch wenn der gesamte Prozess der Code-Review dauert eine lange Zeit, das Vorhandensein einer schnellen Antwort vom Gutachter während des gesamten Prozesses viel leichteres Leben für Entwickler , die durch die „langsame“ Antwort gereizt fühlen.

Wenn Sie unmittelbar nach Erhalt der Anfrage zu beschäftigt für eine vollständige Überprüfung sind, können Sie dennoch schnell mit einer Nachricht über die Fristen antworten oder die Überprüfung anderen Überprüfern anbieten, die schneller antworten können, oderGeben Sie einige erste allgemeine Kommentare (Hinweis: Nichts davon bedeutet, dass Sie die Codierung unterbrechen müssen, um eine solche Antwort zu senden - senden Sie die Antwort an einem angemessenen Haltepunkt in Ihrer Arbeit).

Es ist wichtig, dass die Prüfer genügend Zeit mit der Prüfung verbringen und sicherstellen, dass ihr „LGTM“ bedeutet, dass „dieser Code unseren Standards entspricht “. Individuelle Antworten sollten jedoch im Idealfall ohnehin schnell sein .

Codeüberprüfung zwischen Zeitzonen

Versuchen Sie beim Arbeiten zwischen verschiedenen Zeitzonen, dem Autor zu antworten, während er noch im Büro ist. Wenn er bereits nach Hause gegangen ist, versuchen Sie unbedingt, eine Antwort zu senden, bevor er am nächsten Tag ins Büro zurückkehrt.

Reservierte LGTM

Aus Geschwindigkeitsgründen gibt es bestimmte Situationen, in denen der Prüfer LGTM / Genehmigung erteilen muss, selbst bei unbeantworteten Kommentaren zum CL. Dies geschieht, wenn:

• Der Prüfer ist zuversichtlich, dass der Entwickler alle verbleibenden Kommentare ordnungsgemäß berücksichtigt.
  
• Andere Änderungen sind geringfügig und optional .

Der Prüfer muss angeben, welche dieser Optionen er meint, wenn dies nicht klar ist.

Reservierte LGTMs sind besonders nützlich, wenn sich Entwickler und Prüfer in unterschiedlichen Zeitzonen befinden. Andernfalls wartet der Entwickler den ganzen Tag, bis er die LGTM-Genehmigung erhält.

Großer CL

Wenn Ihnen jemand einen Überprüfungscode sendet, der so groß ist, dass Sie nicht sicher sind, wann Sie ihn anzeigen können, besteht die typische Antwort darin, den Entwickler zu bitten, den CL in mehrere kleinere CLs aufzuteilen . Dies ist normalerweise möglich und für Prüfer sehr nützlich, auch wenn der Entwickler zusätzliche Arbeit benötigt.

Wenn CL nicht in kleinere CLs zerlegt werden kann und Sie keine Zeit haben, dies alles schnell durchzusehen, schreiben Sie zumindest einige Kommentare zum gesamten CL-Design und senden Sie sie zur Verbesserung an den Entwickler zurück. Eines Ihrer Ziele als Prüfer sollte es immer sein, den Entwickler „freizuschalten“ oder ihm zu ermöglichen, schnell weitere Maßnahmen zu ergreifen, ohne die Qualität des Codes zu beeinträchtigen.

Verbesserungen der Codeüberprüfung im Laufe der Zeit

Wenn Sie diese Empfehlungen befolgen und sich strikt der Codeüberprüfung nähern, werden Sie feststellen, dass sich der gesamte Prozess im Laufe der Zeit beschleunigt und beschleunigt. Entwickler werden herausfinden, was für qualitativ hochwertigen Code erforderlich ist, und Ihnen CLs senden, die von Anfang an großartig sind und immer weniger Zeit zum Anzeigen benötigen. Prüfer lernen, schnell zu reagieren und den Überprüfungsprozess nicht unnötig zu verzögern. Kompromittieren Sie jedoch nicht die Standards oder die Qualität der Codeüberprüfung, um eine imaginäre Geschwindigkeitsverbesserung zu erzielen. Tatsächlich erzielen Sie auf lange Sicht keine Gesamtbeschleunigung.

Notsituationen

Es gibt auch Notfallsituationen, in denen CLs den gesamten Codeüberprüfungsprozess sehr schnell durchlaufen müssen und in denen Qualitätsprinzipien abgeschwächt werden müssen. Lesen Sie jedoch bitte die Beschreibung, welche Situationen als Notfall gelten und welche nicht.

So schreiben Sie Kommentare in die Codeüberprüfung

Zusammenfassung

• Sei höflich.
  
• Erklären Sie Ihre Argumentation.
  
• Vermeiden Sie explizite Bestellungen, indem Sie einfach auf Probleme hinweisen und den Entwickler entscheiden lassen.
  
• Ermutigen Sie Entwickler, Code zu vereinfachen oder Kommentare anstelle einfacher Erklärungen für die Komplexität hinzuzufügen.

Höflichkeit

Im Allgemeinen ist es wichtig, höflich und respektvoll zu sein und dem Entwickler, dessen Code Sie anzeigen, sehr klar und hilfreich zu sein. Eine Möglichkeit, dies zu tun, besteht darin, sicherzustellen, dass Sie immer Kommentare zum Code und niemals zum Entwickler abgeben . Sie müssen diese Praxis nicht immer befolgen, aber Sie müssen sie anwenden, wenn Sie etwas Unangenehmes oder Kontroverses sagen. Zum Beispiel:

Schlecht: "Warum haben Sie hier Streams verwendet, wenn es offensichtlich ist, dass Parallelität keinen Nutzen bringt?"

Gut: „Das Parallelitätsmodell hier erhöht die Komplexität des Systems, und ich sehe keinen tatsächlichen Leistungsvorteil. Da es keinen Leistungsvorteil gibt, ist es am besten, wenn dieser Code Single-Threaded ist und nicht mehrere Threads verwendet. “

Erklären Sie die Gründe

Im obigen „guten“ Beispiel können Sie sehen, dass es dem Entwickler hilft, zu verstehen, warum Sie Ihren Kommentar abgeben. Sie müssen diese Informationen nicht immer in Ihre Kommentare aufnehmen, aber manchmal ist es angebracht, etwas mehr Erläuterungen zu Ihrer Logik, den von Ihnen befolgten Best Practices oder zur Verbesserung der Qualität des Codes durch Ihren Vorschlag zu geben.

Richtungen

Im Allgemeinen ist es Aufgabe des Entwicklers, Korrekturen vorzunehmen, nicht des Prüfers. Sie müssen keinen detaillierten Lösungsentwurf entwickeln oder Code für den Entwickler schreiben.

Dies bedeutet jedoch nicht, dass der Prüfer hier nicht helfen kann. Im Allgemeinen sollte ein angemessenes Gleichgewicht zwischen der Ermittlung von Problemen und der Bereitstellung direkter Hilfe hergestellt werden. Das Aufzeigen von Problemen und die Möglichkeit für den Entwickler, eine Entscheidung zu treffen, hilft dem Entwickler häufig beim Lernen und erleichtert die Codeüberprüfung. Dies kann auch zu einer besseren Lösung führen, da der Entwickler näher am Code ist als der Prüfer.

Direkte Anweisungen, Vorschläge oder sogar Code sind jedoch manchmal nützlicher. Der Hauptzweck einer Codeüberprüfung besteht darin, die bestmögliche CL zu erhalten. Das sekundäre Ziel besteht darin, die Fähigkeiten der Entwickler zu verbessern, damit die Überprüfung für sie immer weniger Zeit in Anspruch nimmt.

Akzeptiere die Erklärung

Wenn Sie den Entwickler bitten, einen unverständlichen Code zu erklären, führt dies normalerweise dazu, dass er ihn klarer umschreibt . Manchmal ist das Hinzufügen eines Kommentars auch eine angemessene Antwort, wenn es nicht nur eine Erklärung für zu komplexen Code ist.

Erklärungen, die nur im Überprüfungstool geschrieben wurden, helfen zukünftigen Lesern nicht. Sie sind nur in einigen Fällen akzeptabel, beispielsweise wenn Sie sich einen Bereich ansehen, mit dem Sie nicht sehr vertraut sind, und der Entwickler erklärt, was gewöhnliche Codeleser bereits wissen sollten.

So überwinden Sie Widerstände bei der Codeüberprüfung

Manchmal argumentiert ein Entwickler in einem Codeüberprüfungsprozess. Entweder stimmt er Ihrem Vorschlag nicht zu oder er beschwert sich, dass Sie im Allgemeinen zu streng sind.

Wer hat recht?

Wenn der Entwickler Ihrem Vorschlag nicht zustimmt, nehmen Sie sich zunächst die Zeit, um seine Position zu prüfen. Oft ist es näher an Ihrem Code und hat daher möglicherweise eine bessere Vorstellung von einigen Aspekten. Macht es in seiner Argumentation Sinn? Ist es in Bezug auf die Codequalität sinnvoll? Wenn ja, dann geben Sie zu, dass er Recht hat, und die Frage wird verschwinden.

Entwickler haben jedoch nicht immer Recht. In diesem Fall muss der Prüfer weiter erläutern, warum er der Ansicht ist, dass sein Vorschlag korrekt ist. Eine gute Erklärung zeigt sowohl ein Verständnis der Reaktion des Entwicklers als auch zusätzliche Informationen darüber, warum die Änderung erforderlich ist.

Insbesondere wenn der Prüfer der Ansicht ist, dass sein Vorschlag die Qualität des Codes verbessern wird, sollte er darauf bestehen, wenn er der Ansicht ist, dass die daraus resultierende Qualitätsverbesserung die zusätzliche Arbeit rechtfertigt. Die Verbesserung der Codequalität erfolgt in der Regel in kleinen Schritten.

Manchmal dauert es mehrere Erklärungsrunden, bis es tatsächlich akzeptiert wird. Stellen Sie einfach sicher, dass Sie immer höflich bleiben , und lassen Sie den Entwickler wissen, dass Sie es hören. Stimmen Sie einfach nicht zu.

Über den Groll der Entwickler

Rezensenten haben manchmal das Gefühl, dass ein Entwickler verärgert ist, wenn ein Rezensent auf einer Verbesserung besteht. Manchmal sind Entwickler wirklich verärgert, aber normalerweise nicht lange, und später werden sie sehr dankbar sein, dass Sie ihnen geholfen haben, die Qualität ihres Codes zu verbessern. Wenn Sie in Ihren Kommentaren höflich sind, sind die Entwickler normalerweise überhaupt nicht wirklich verärgert, und die Besorgnis liegt nur im Kopf des Rezensenten. Der Stil des Schreibens von Kommentaren ist in der Regel frustrierender als die Beharrlichkeit des Überprüfers in Bezug auf die Qualität des Codes.

Ausstehende Änderungen

Eine typische Quelle für Kontroversen ist, dass Entwickler (aus offensichtlichen Gründen) die Arbeit erledigen möchten. Sie möchten keine weitere Bewertungsrunde für diesen CL durchlaufen. Daher sagen sie, dass sie etwas in der späteren CL entfernen werden, und jetzt müssen Sie LGTM für diese CL erstellen . Einige Entwickler machen das sehr gut und schreiben sofort einen weiteren CL, der das Problem behebt. Die Erfahrung hat jedoch gezeigt, dass diese Bearbeitung umso weniger wahrscheinlich ist, je mehr Zeit nach dem ursprünglichen CL vergeht. In der Tat normalerweise, wenn der Entwickler die Bearbeitung nicht sofort vornimmtdas ist normalerweise nie so. Nicht weil die Entwickler unverantwortlich sind, sondern weil sie viel Arbeit haben und die Bearbeitung unter der Rampe anderer Arbeiten verloren geht oder vergessen wird. Daher ist es normalerweise am besten, auf einer sofortigen Korrektur zu bestehen, bevor das Commit in die Codebasis eingeht und „abgeschlossen“ ist. Das Zulassen ausstehender Änderungen ist eine typische Methode zum Degenerieren von Codebasen.

Wenn der CL eine neue Komplexität einführt, muss diese vor dem Senden behoben werden, wenn dies kein Notfall ist. Wenn CL andere Probleme anzeigt, die derzeit nicht gelöst werden können, sollte der Entwickler einen Fehler im Tracker registrieren und sich selbst zuweisen, damit er nicht verloren geht. Er kann optional einen TODO-Kommentar zu diesem Fehler in den Code schreiben.

Allgemeine Beschwerden über die Schwere

Wenn Sie früher eine eher oberflächliche Codeüberprüfung durchgeführt und in den strengen Modus gewechselt haben, werden sich einige Entwickler sehr laut beschweren. Eine erhöhte Geschwindigkeit der Codeüberprüfung löst diese Beschwerden normalerweise.

Manchmal kann es Monate dauern, bis Beschwerden verschwinden, aber am Ende neigen Entwickler dazu, den Wert strenger Codeüberprüfungen zu erkennen, wenn sie sehen, wie großartig der Code ist. Manchmal werden die lautesten Demonstranten sogar zu Ihren stärksten Unterstützern, wenn etwas passiert, das sie wirklich den Wert strenger Überprüfungen erkennen lässt.

Konfliktlösung

Wenn Sie alle oben genannten Schritte ausführen, aber dennoch auf Konflikte stoßen, finden Sie im Abschnitt Code Review Standard Empfehlungen und Grundsätze, die zur Lösung des Konflikts beitragen können.