So machen Sie Codeüberprüfungen schneller und effizienter

Wie erfolgt normalerweise eine Codeüberprüfung? Sie senden eine Poolanfrage, erhalten Feedback, nehmen Korrekturen vor, senden Korrekturen für eine zweite Überprüfung, erhalten dann die Genehmigung und führen eine Zusammenführung durch. Es klingt einfach, aber in Wirklichkeit kann der Überprüfungsprozess sehr zeitaufwändig sein.

Stellen Sie sich vor, Sie haben eine Poolanfrage mit Hunderten von Änderungszeilen. Der Prüfer sollte viel Zeit darauf verwenden, den Code vollständig zu lesen und die vorgeschlagenen Änderungen zu verstehen. Infolgedessen kann der gesamte Prozess von der Erstellung einer Poolanforderung bis zu ihrer Genehmigung mehrere Tage dauern - dies ist sowohl für den Prüfer als auch für den Autor der Änderungen nicht sehr angenehm. Und die Chancen stehen gut, dass der Rezensent am Ende noch etwas verpasst. Oder die Prüfung ist möglicherweise zu oberflächlich, und im schlimmsten Fall kann die Poolanforderung sofort abgelehnt werden.

Es stellt sich heraus, dass die Überprüfung umso geringer ist, je größer die Poolanforderung ist.

Wie vermeide ich solche Situationen? Wie kann die Poolanfrage für den Prüfer einfacher und verständlicher gemacht und der gesamte Prozess optimiert werden?

Wir übersetzen einen Artikel unseres Backend-Entwicklers Sergey Zhuk über die Funktionsweise des Codeüberprüfungsprozesses des Skyeng-Teams für mobile Anwendungen.

Kategorien ändern

Stellen wir uns vor, Sie haben eine Aufgabe - neue Funktionen im Projekt zu implementieren. Die Poolanforderung, an der Sie arbeiten, kann verschiedene Kategorien von Änderungen enthalten. Natürlich wird es einen neuen Code geben. Im Laufe der Arbeit stellen Sie jedoch möglicherweise fest, dass einige Codes im Voraus überarbeitet werden müssen, damit neue Funktionen hinzugefügt werden können. Oder mit dieser neuen Funktionalität wurde eine Duplizierung in dem Code angezeigt, den Sie entfernen möchten. Oder Sie haben plötzlich einen Fehler gefunden und möchten ihn beheben. Wie soll die endgültige Poolanfrage aussehen?

Schauen wir uns zunächst an, welche Kategorien von Änderungen mit Code auftreten können.

1. Funktionsänderungen.
2. Strukturelles Refactoring - Änderungen an Klassen, Schnittstellen, Methoden und Bewegungen zwischen Klassen.
3. Einfaches Refactoring - kann mithilfe der IDE durchgeführt werden (z. B. Extrahieren von Variablen / Methoden / Konstanten, Vereinfachen von Bedingungen).
4. Klassen umbenennen und verschieben - Namespace neu organisieren.
5. Nicht verwendeten (toten) Code entfernen.
6. Korrekturcode-Stil.

Strategieüberprüfung

Es ist sehr wichtig zu verstehen, dass jede dieser Kategorien unterschiedlich überprüft wird, und wenn der Prüfer sie berücksichtigt, sollte er sich auf verschiedene Dinge konzentrieren. Man kann sagen, dass jede Kategorie von Änderungen ihre eigene Überprüfungsstrategie beinhaltet.

1. Funktionsänderung: Lösung von Geschäftsproblemen und Systemdesign.
2. Strukturelles Refactoring: Abwärtskompatibilität und Designverbesserung.
3. Primitives Refactoring: Verbesserte Lesbarkeit. Diese Änderungen können hauptsächlich mithilfe der IDE vorgenommen werden (z. B. Extrahieren von Variablen / Methoden / Konstanten usw.).
4. Klassen umbenennen / verschieben: Hat sich die Namespace-Struktur verbessert?
5. Nicht verwendeten Code entfernen: abwärtskompatibel.
6. Korrekturcodestil: Meistens erfolgt die Zusammenführungspoolanforderung sofort.

Es werden verschiedene Ansätze verwendet, um Änderungen in verschiedenen Kategorien zu überprüfen. Daher variiert auch der Zeit- und Arbeitsaufwand für deren Überprüfung.

Funktionsänderungen. Dies ist der längste Prozess, da die Domänenlogik geändert werden muss. Der Prüfer prüft, ob das Problem behoben ist, und prüft, ob die vorgeschlagene Lösung am besten geeignet ist oder verbessert werden kann.

Strukturelles Refactoring. Dieser Prozess benötigt viel weniger Zeit als funktionale Änderungen. Es kann jedoch Vorschläge und Meinungsverschiedenheiten darüber geben, wie genau der Code organisiert werden soll.

Wenn in 99% der Fälle die verbleibenden Kategorien überprüft werden, erfolgt die Zusammenführung sofort.

1. Einfaches Refactoring. Ist der Code lesbarer geworden? - Merzhim.
2. Klassen umbenennen / verschieben. Wurde die Klasse in einen besseren Namespace verschoben? - Merzh.
3. Das Entfernen von nicht verwendetem (totem) Code ist merzhim.
4. Korrekturen Codestil oder Formatierung - merzh. Ihre Kollegen sollten dies während der Codeüberprüfung nicht überprüfen, dies ist die Aufgabe des Linter.

Warum sollten Änderungen kategorisiert werden?

Wir haben bereits diskutiert, dass verschiedene Kategorien von Änderungen unterschiedlich überarbeitet werden. Beispielsweise überprüfen wir funktionale Änderungen basierend auf den Geschäftsanforderungen und beim strukturellen Refactoring die Abwärtskompatibilität. Und wenn wir mehrere Kategorien mischen, wird es für den Prüfer schwierig sein, mehrere Prüfungsstrategien gleichzeitig im Auge zu behalten. Und höchstwahrscheinlich wird der Prüfer mehr Zeit als nötig mit der Poolanforderung verbringen, und aus diesem Grund kann er etwas verpassen. Wenn die Poolanforderung verschiedene Arten von Änderungen enthält, muss der Prüfer bei jeder Korrektur alle diese Kategorien erneut überarbeiten. Zum Beispiel haben Sie strukturelles Refactoring und funktionale Änderungen gemischt. Auch wenn das Refactoring gut durchgeführt wird, aber ein Problem mit der Implementierung der Funktion vorliegt, muss der Prüfer nach den Korrekturen von Anfang an die gesamte Poolanforderung prüfen. Überprüfen Sie sowohl das Refactoring als auch die Funktionsänderungen erneut. Der Prüfer verbringt also mehr Zeit mit der Poolanforderung. Anstatt sofort eine separate Poolanforderung mit Refactoring in Betracht zu ziehen, sollte der Prüfer den gesamten Code erneut überprüfen.

Was genau ist es nicht wert, gemischt zu werden

Umbenennen / Löschen einer Klasse und deren Umgestaltung. Hier stoßen wir auf Git, das solche Änderungen nicht immer richtig versteht. Ich meine große Änderungen, wenn sich viele Zeilen ändern. Wenn Sie eine Klasse umgestalten und dann irgendwohin verschieben, nimmt Git sie nicht als bewegend wahr. Stattdessen interpretiert Git diese Änderungen so, dass eine Klasse entfernt und eine andere erstellt wird. Dies führt zu einer Reihe von Fragen während der Codeüberprüfung. Und der Autor des Codes wird gefragt, warum er diesen hässlichen Code geschrieben hat, obwohl dieser Code mit geringfügigen Änderungen einfach von einem Ort zum anderen verschoben wurde.

Alle funktionalen Änderungen + jegliches Refactoring. Wir haben diesen Fall bereits oben besprochen. Dies führt dazu, dass der Prüfer zwei Überprüfungsstrategien gleichzeitig im Auge behält. Selbst wenn das Refactoring gut durchgeführt wird, können wir diese Änderungen nicht verzögern, bis funktionale Änderungen genehmigt werden.

Alle mechanischen Änderungen + alle vom Menschen vorgenommenen Änderungen. Mit "mechanischen Änderungen" meine ich jede Formatierung, die mit der IDE oder der Codegenerierung durchgeführt wird. Zum Beispiel wenden wir den neuen Codestil an und erhalten 3000 Zeilenänderungen. Und wenn wir diese Änderungen mit funktionalen oder anderen von einer Person vorgenommenen Änderungen mischen, werden wir den Prüfer zwingen, diese Änderungen und Gründe mental zu klassifizieren: Dies ist eine Änderung, die von einem Computer vorgenommen wird - sie kann übersprungen werden, und diese von einer Person vorgenommene Änderung ist erforderlich auschecken. Der Prüfer verbringt also viel zusätzliche Zeit mit der Überprüfung.

Beispiel

Hier ist eine Poolanforderung mit einer Methodenfunktion, die die Clientverbindung zu Memcached sanft schließt:



Selbst wenn die Poolanforderung klein ist und nur hundert Codezeilen enthält, ist es immer noch schwierig, sie zu überprüfen. Wie Sie den Commits entnehmen können, enthält es verschiedene Kategorien von Änderungen:

• funktional (neuer Code),
• Refactoring (Erstellen / Verschieben von Klassen),
• Korrekturcode-Stil (Entfernen überschüssiger Dockblöcke).

Gleichzeitig umfasst die neue Funktionalität selbst nur 10 Zeilen:



Infolgedessen sollte der Prüfer den gesamten Code und überprüfen

• Stellen Sie sicher, dass das Refactoring in Ordnung ist.
• Überprüfen Sie, ob die neue Funktionalität korrekt implementiert ist.
• Stellen Sie fest, ob diese Änderung automatisch von der IDE oder von der Person generiert wurde.

Daher ist eine solche Poolanforderung schwer zu überprüfen. Um die Situation zu korrigieren, können Sie diese Commits in separate Zweige aufteilen und für jeden einen Pool von Anforderungen erstellen.

1. Refactoring: Klassenextraktion

Es gibt nur zwei Dateien. Der Prüfer sollte nur das neue Design überprüfen. Wenn alles in Ordnung ist - Merzhim.

2. Der nächste Schritt ist auch das Refactoring. Wir verschieben die beiden Klassen einfach in einen neuen Namespace

Eine solche Poolanforderung ist recht einfach zu überprüfen und kann sofort instanziiert werden.

3. Überschüssige Dockblöcke entfernen

Nichts interessantes hier. Merzhim.

4. Die Funktion selbst

Und jetzt enthält die Poolanforderung mit Funktionsänderungen nur noch den gewünschten Code. Ihr Prüfer kann sich also nur auf diese Aufgabe konzentrieren. Die Poolanfrage ist klein und leicht zu überprüfen.

Fazit

Faustregel:

Erstellen Sie keine großen Poolanforderungen mit gemischten Änderungskategorien.

Je größer die Poolanforderung ist, desto schwieriger ist es für den Prüfer, die von Ihnen vorgeschlagenen Änderungen zu verstehen. Höchstwahrscheinlich wird eine große Poolanfrage mit Hunderten von Codezeilen abgelehnt. Teilen Sie es stattdessen in kleine logische Teile auf. Wenn Ihr Refactoring in Ordnung ist, Funktionsänderungen jedoch Fehler enthalten, kann das Refactoring leicht zurückgehalten werden, sodass Sie und Ihr Prüfer sich auf die Funktionalität konzentrieren können, ohne den gesamten Code von Anfang an zu betrachten.

Befolgen Sie immer die folgenden Schritte, bevor Sie die Poolanforderung senden:

• Optimieren Sie Ihren Code zum Lesen. Code ist viel besser lesbar als geschrieben;
• Beschreiben Sie die vorgeschlagenen Änderungen, um den notwendigen Kontext für ihr Verständnis bereitzustellen.
• Überprüfen Sie immer Ihren Code, bevor Sie eine Poolanforderung erstellen. Und tun Sie es so, als wäre es der Code eines anderen. Manchmal hilft es, etwas zu finden, das Sie verpasst haben. Dies verringert die Wahrscheinlichkeit der Ablehnung Ihrer Poolanforderung und die Anzahl der Korrekturen.

Mein Kollege Oleg Antonevich erzählte mir von der Idee, Änderungen in Kategorien zu unterteilen.

Vom Herausgeber: Sergey schreibt viele interessante Dinge über Programmierung und PHP, und manchmal übersetzen wir etwas: einen Streaming-Videoserver , der HTML-Dateien rendert . Fühlen Sie sich frei, ihm Fragen in den Kommentaren zu diesem Artikel zu stellen - er wird antworten!

Wir erinnern Sie auch daran, dass wir immer viele interessante Stellen für Entwickler haben!