Gute Richtlinien und Praktiken für die obligatorische Codeüberprüfung [geschlossen]

Wir probieren für ein paar Sprints eine obligatorische Codeüberprüfung bei jedem Commit aus - nichts gelangt in den Master, der nicht von mindestens einer Person, nicht dem Autor, validiert wurde. Wir haben uns sowohl von Entwicklern als auch vom Management eingekauft (was eine erstaunliche Situation ist) und möchten einige der Vorteile nutzen, für die es bekannt ist:

• offensichtliche Fehlerreduzierung
• mehr Bewusstsein für Veränderungen rund um das Projekt
• der "Ich weiß, dass sich jemand das ansehen wird, damit ich nicht faul bin" / Anti-Cowboy-Effekt
• Erhöhte Konsistenz innerhalb / zwischen Projekten

Aber wir , die etwas vorstellen , das ist bekannt Geschwindigkeit zu reduzieren, und wenn falsch gemacht könnte einen dummen bürokratischen Schritt schafft in der Pipeline zu begehen, der nichts tun , aber die Zeit in Anspruch nehmen. Dinge, die mir Sorgen machen:

• Bewertungen, die sich nur auf die Auswahl konzentrieren
• (hyperbolisch) Menschen, die im Rahmen einer zweizeiligen Commit-Überprüfung große architektonische Probleme aufwerfen.
• Ich möchte Antworten nicht mit anderen Dingen verzerren.

Obwohl wir alle vernünftige Leute sind und eine Menge Selbstanalysen durchführen, könnten wir definitiv einen kampferprobten Einblick in die Art von Dingen gebrauchen, die wir in einer Überprüfungssitzung erreichen sollten, damit Überprüfungen wirklich für uns funktionieren . Welche Richtlinien und Richtlinien haben sich bewährt?

1. Machen Sie Bewertungen kurz.

   Es ist schwierig, lange Zeit konzentriert zu bleiben, insbesondere während der Codeüberprüfung. Darüber hinaus können lange Codeüberprüfungen darauf hinweisen, dass der Code entweder zu viel zu sagen hat (siehe die nächsten beiden Punkte) oder dass die Überprüfung zu einer Diskussion über größere Probleme wie die Architektur wird.

   Eine Überprüfung sollte auch eine Überprüfung bleiben, keine Diskussion. Dies bedeutet nicht, dass der Autor des Codes nicht antworten kann, aber es sollte nicht zu einem langen Meinungsaustausch führen.

2. Vermeiden Sie es, zu schlechten Code zu überprüfen.

   Das Überprüfen von Code geringer Qualität ist sowohl für den Prüfer als auch für den Autor des Codes bedrückend. Wenn ein Code schrecklich ist, ist eine Codeüberprüfung nicht hilfreich. Stattdessen sollte der Autor aufgefordert werden, den Code korrekt umzuschreiben.

3. Verwenden Sie vor der Überprüfung automatische Prüfer.

   Automatisierte Prüfer vermeiden es, wertvolle Zeit damit zu verschwenden, auf Fehler hinzuweisen, die automatisch gefunden werden können. Für C # -Code ist beispielsweise das Ausführen von StyleCop, Codemetriken und insbesondere der Codeanalyse eine gute Gelegenheit, einige der Fehler vor der Überprüfung zu finden. Dann kann die Codeüberprüfung für Punkte ausgegeben werden, die für eine Maschine äußerst schwierig sind.

4. Wählen Sie sorgfältig die Personen aus, die Bewertungen durchführen.

   Zwei Personen, die sich nicht ertragen können, werden den Code eines anderen nicht gut überprüfen. Das gleiche Problem tritt auf, wenn eine Person die andere nicht respektiert (egal ob es sich übrigens um den Rezensenten oder den Autor handelt).

   Einige Personen können ihren Code auch nicht überprüfen lassen. Daher benötigen sie spezielle Schulungen und Vorbereitungen, um zu verstehen, dass sie nicht kritisiert werden und ihn nicht als etwas Negatives ansehen sollten. Eine unvorbereitete Überprüfung wird nicht helfen, da sie immer in der Defensive sind und keine Kritiker ihres Codes hören (wobei jeder Vorschlag als Kritik aufgefasst wird).

5. Führen Sie sowohl informelle als auch formelle Überprüfungen durch.

   Eine Checkliste hilft dabei, sich auf eine Reihe von Fehlern zu konzentrieren und zu vermeiden, dass man sich auf das Sammeln von Fehlern konzentriert. Diese Checkliste kann Punkte enthalten wie:

   • SQL-Injektion,
   • Falsche Annahmen über eine Sprache, die zu Fehlern führen können,
   • Bestimmte Situationen, die zu Fehlern führen können, z. B. die Priorität des Bedieners. In C # var a = b ?? 0 + c ?? 0;sieht es beispielsweise für jemanden gut aus, der zwei nullfähige Zahlen mit einer Koaleszenz von Null hinzufügen möchte, dies ist jedoch nicht der Fall.
   • Freigabe der Erinnerung,
   • Faules Laden (mit seinen zwei Risiken: das gleiche Ding mehr als einmal laden und es überhaupt nicht laden),
   • Überläufe,
   • Datenstrukturen (mit Fehlern wie beispielsweise einer einfachen Liste anstelle eines Hash-Sets),
   • Eingabevalidierung und defensive Programmierung im Allgemeinen,
   • Gewindesicherheit,
   • etc.

   Ich stoppe die Liste hier, aber es gibt Hunderte von Punkten, die in einer Checkliste enthalten sein können, abhängig von den Schwachstellen eines bestimmten Autors.

6. Passen Sie die Checkliste schrittweise an.

   Um im Laufe der Zeit konstruktiv und nützlich zu bleiben, sollten die in formellen Überprüfungen verwendeten Checklisten im Laufe der Zeit abhängig von den gefundenen Fehlern angepasst werden. Beispielsweise können erste informelle Überprüfungen ein gewisses Risiko für SQL Injection aufzeigen. Die SQL Injection-Prüfung wird in die Checkliste aufgenommen. Wenn der Autor einige Monate später den Eindruck hat, dass er bei dynamischen und parametrisierten Abfragen sehr vorsichtig ist, wird SQL Injection möglicherweise aus der Checkliste entfernt.

Wir haben fast wie eine Checkliste:

• Zeigen Sie mir die Aufgabenbeschreibung.
• Führen Sie mich durch das Ergebnis und zeigen Sie, dass es funktioniert. Führen Sie verschiedene Szenarien aus (ungültige Eingabe usw.).
• Zeigen Sie mir die bestandenen Tests. Wie ist die Testabdeckung?
• Zeigen Sie mir den Code - hier suchen wir nach offensichtlichen Ineffizienzen.

Funktioniert ziemlich gut.

Ich denke, eine Person, die die Macht über die anderen hat, würde ausreichen, Administrator oder Moderator, um irrelevante Kommentare zu schneiden und die Überprüfung von Dingen zu beschleunigen, die eine schnelle Überprüfung erfordern. Einzelentscheider.

Ein Minus davon wäre, dass diese Person es als Hauptaufgabe tun muss, während sie etwas anderes tun könnte, und wahrscheinlich möchten Sie die erfahrenste Person in dieser Position haben.

Zweitens automatisieren Sie so viel wie möglich!

• Leerzeichen kontrollieren
• Stilsteuerungssoftware
• automatisierte Builds vor der Codeüberprüfung
• automatisierte Tests vor der Codeüberprüfung

Diese Dinge werden zumindest einige Dinge entfernen, die Leute ohne wirkliche Notwendigkeit kommentieren könnten. Wenn es nicht erstellt wird oder nachgestellte Leerzeichen hat, ist es nicht gut genug für die Überprüfung, beheben Sie es und beantragen Sie die Überprüfung erneut. Wenn es nicht erstellt wird oder ein Test fehlschlägt, ist es offensichtlich, dass es nicht gut genug ist.

Vieles hängt von Ihren Technologien ab, aber finden Sie heraus, was Sie automatisch überprüfen können, je mehr desto besser.

Wir haben diesen Kampf noch nicht gewonnen, aber das fanden wir nützlich.