Was sind die Vor- und Nachteile der Klassifizierung von Fehlern während einer Peer-Code-Überprüfung?

Vor ungefähr 3 Monaten hat unsere Engineering-Gruppe das Review Board eingeführt , das für alle Peer-Code-Überprüfungen verwendet werden soll. Heute hatte ich eine Diskussion mit einer der an diesem Prozess beteiligten Personen und fand heraus, dass wir aufgrund mehrerer fehlender Funktionen bereits nach einem Ersatz suchen (möglicherweise etwas Kommerzielles).

Eine der Funktionen, die anscheinend von vielen Menschen gefragt wird, ist die Fähigkeit, jeden Codeüberprüfungskommentar zu klassifizieren / zu kategorisieren (dh es handelt sich um ein Stilproblem, eine Codierungskonvention, ein Ressourcenleck, einen Logikfehler, einen Absturz ... was auch immer).

Ist diese Kategorisierung für die Teams, die regelmäßig die Codeüberprüfung üben, eine gängige Praxis? Machst du es? Hast du es in der Vergangenheit getan? Ist es gut / schlecht?

Einerseits gibt es dem Team mehr Metriken und zeigt möglicherweise spezifischere Bereiche an, in denen Entwickler möglicherweise geschult werden müssen (zumindest scheint dies das Argument zu sein). Gibt es noch andere Vorteile? Auf der anderen Seite, und das ist meine Sorge, wird der Codeüberprüfungsprozess dadurch noch viel langsamer. Als Teamleiter habe ich einen ziemlich großen Teil der Überprüfungen durchgeführt, und mir hat immer die Möglichkeit gefallen, einen Teil des Codes hervorzuheben, einen Kommentar abzuhämmern und so schnell wie möglich fortzufahren. Obwohl ich es nicht persönlich ausprobiert habe, habe ich das Gefühl, dass sich jedes Mal, wenn Sie dieses Kombinationsfeld erweitern und nach der richtigen Kategorie scrollen / suchen, so anfühlt, als würde Sie etwas stolpern. Auch wenn wir anfangen, Metriken für dieses Zeug zu behalten,

Ich glaube, dass Sie Ihre eigene Frage nach den relativen Vor- und Nachteilen weitgehend beantwortet haben.

Codeüberprüfungen können oft recht mühsam und langwierig sein, und alles, was die Zeit für die Überprüfung von Code verlängert, wird es noch schlimmer erscheinen lassen. So wie ich das sehe, möchten Sie Ihren Überprüfungsprozess sehr kurz halten und nicht zu stark über die Feinheiten nachdenken. Der Schlüssel ist also zu entscheiden, was es mit dem Überprüfungsprozess auf sich hat, das Ihrem Team geschäftlichen Nutzen bringt.

Stilfragen sind wahrscheinlich einer der Punkte, die ich als niedrigste Priorität bezeichnen würde. Sicher, wenn Sie den Code sauber und einheitlich formatieren, kann dies das Verständnis erleichtern, aber die Aufregung um den Stil kann auch zu RIESIGEN Ineffizienzen während des Codierungsprozesses führen, da die Entwickler sich Gedanken darüber machen, wie hübsch der Code ist, um die zu lösenden Probleme zu lösen. Wenn Sie immer noch über Stilprobleme besorgt sind, ist die Verwendung eines Tools zur Überprüfung von Stil / Formatierung (z. B. StyleCop für C #) eine gute Möglichkeit, die stilspezifischen Probleme dem letzten Moment zu überlassen und den Entscheidungsprozess in Bezug auf den Stil zu übernehmen aus den Händen der Entwickler, um ihr Denken für die wichtigeren Dinge zu befreien. Wenn Sie kein solches Produkt für die Sprache Ihrer Wahl haben, kann möglicherweise ein einfacher Parser geschrieben werden, um Ihren Code schnell nach solchen Problemen zu durchsuchen.

Speicherlecks und andere leistungsspezifische Probleme sollten niemals dem Überprüfungsprozess überlassen bleiben. Sicher, wenn Sie etwas entdecken, das offensichtlich ein großes Problem verursacht, sollten Sie darauf hinweisen, aber es sollte nicht der Zweck der Codeüberprüfung sein, jedes kleine Speicher- / Leistungsproblem in Ihrem Code aufzuspüren. Dafür ist ein gutes Profiling-Tool gedacht, und sie sind jeden Cent wert, wenn Sie es schaffen, ein sehr gutes für die Sprache zu finden, in der Sie sich entwickeln.

Logikprobleme sind bestenfalls problematisch, und dies sind die Dinge, die beim Überprüfen von Code wirklich viel wertvolle Zeit in Anspruch nehmen können. Anstatt dies alles ganz der Codeüberprüfung zu überlassen, sollten Ihre Unit-Tests dafür verwendet werden. Ja, auch Tests können falsch sein. Wenn Sie jedoch zuerst einen Test entwickeln, sich an die SRP- und DRY-Prinzipien halten, gnadenlos umgestalten und Ihre Komponententests als Mittel zur Validierung Ihrer Spezifikationen definieren, werden Sie feststellen, dass Sie am Ende davon betroffen sind weit weniger logische Probleme. Wenn Sie nach dem Codieren testen, ist es weniger wahrscheinlich, dass Sie sich mit potenziellen Logikproblemen befassen, sobald diese auftreten, und Sie vergessen eher, einen bestimmten Pfad durch Ihren Code zu testen.

Wenn Sie also alles tun, was ich hier vorgeschlagen habe, was müssen Sie dann in der Codeüberprüfung tun? Die einfache Antwort lautet, dass Ihre Codeüberprüfung zu einem relativ einfachen Prozess wird, bei dem der Codierer dem Prüfer erklärt, wie eine bestimmte Anforderung in den Tests erfasst wurde und wie diese Tests auf die gelösten Probleme angewendet wurden. Sie neigen dazu, den Code genauer zu überprüfen und die Tests gründlicher zu analysieren, da hier der größte Geschäftswert gemessen werden kann, insbesondere wenn dieser Code später beibehalten werden muss. Um die Überprüfung des Testcodes noch schmerzloser zu gestalten, kann die Verwendung eines guten verhaltensgesteuerten Testframeworks die Überprüfung erheblich vereinfachen, da die Spezifikationen im Code als fast einfache englische Beschreibung der Testausführung erfasst werden. Detaillierte Überprüfungen werden für jeden Code durchgeführt, der die Tests unterstützt. und wenn Ihr BDD-Framework nette Textberichte erstellt, in denen die Tests in einfachen Textstorys / Feature-Anweisungen aufgeführt sind, wird der Prozess noch einfacher. Dies alles führt zu einem äußerst effizienten Prozess, der genauso wertvoll ist wie eine herkömmliche Codeüberprüfung, aber viel schneller und gezielter durchgeführt werden kann, um zu vermeiden, dass Sie sich in Kleinigkeiten und Doppelprüfungen festsetzen das führt dich oft nirgendwo hin. Dieser schlankere Ansatz bedeutet mehr Zeit für das Testen und Codieren und weniger Zeit für Verwaltungsprozesse. Sie können jedoch viel schneller und zielgerichteter durchgeführt werden, um zu vermeiden, dass Sie sich in Kleinigkeiten und Doppelkontrollen festsetzen, die Sie oft nirgendwohin führen. Dieser schlankere Ansatz bedeutet mehr Zeit für das Testen und Codieren und weniger Zeit für Verwaltungsprozesse. Sie können jedoch viel schneller und zielgerichteter durchgeführt werden, um zu vermeiden, dass Sie sich in Kleinigkeiten und Doppelkontrollen festsetzen, die Sie oft nirgendwohin führen. Dieser schlankere Ansatz bedeutet mehr Zeit für das Testen und Codieren und weniger Zeit für Verwaltungsprozesse.

Was ist dann mit diesen Metriken? Wenn alle Probleme einfach als "Fehler" behandelt werden, müssen Sie sich nur mit der Anzahl der Fehler befassen, die vor und nach der Veröffentlichung im Laufe der Zeit auftreten, und ob die identifizierten Fehler in eine bestimmte Richtung tendieren. Wenn Sie auch nur ein halbwegs anständiges Problemverfolgungssystem verwenden, stehen Ihnen all diese Informationen praktisch zur Verfügung, und Sie müssen sich keine Gedanken darüber machen, ob Ihr Überprüfungsprozess sie identifizieren muss. Letztendlich möchte Ihr Team das tun, was es kann, nämlich Software schreiben, und nicht zu viel Zeit mit administrativen Problemen verbringen, die oft nur 1 oder 2 Personen im Unternehmen von Interesse sind.

... auf der anderen Seite, und das ist meine Sorge, ist, dass es den Codeüberprüfungsprozess verlangsamen wird ...

Wenn Sie die Bewertungen leicht halten möchten, sollten mühsame Dinge wie Nachforschungen, Korrekturen und Tests auf entdeckte Fehler nicht vorhanden sein.

• Die einfache ( dumme, wenn Sie möchten) Option, dies zu handhaben, ist, sie einfach fallen zu lassen und zu warten, bis die Tester den Fehler selbst finden und melden. Der Hauptnachteil dieses Ansatzes besteht darin, dass einige Probleme, die im Quellcode leicht sichtbar sind, beim Black-Box-Testen möglicherweise viel Aufwand erfordern.
   
  Threading-Probleme und Speicherlecks fallen häufig in diese Kategorie. Ein Datenrennen, bei dem ich eine Minute brauche, um es anhand des Codes zu erkennen, kann monatelang dort sitzen und alle Tests bestehen, bevor es seinen hässlichen Kopf zeigt.

Aus diesem Grund verwende ich niemals eine Option wie oben und empfehle sie nicht.

• Eine andere Möglichkeit besteht darin, schwere Arbeit aus der Warteschlange zu entfernen - in dem Sinne, dass meine Prüfer- Arbeitswarteschlange woanders abläuft. Diese Option verwende ich, mit der ich mich sehr wohl fühle.
   
  Der Ansatz ist recht einfach. Wenn ich der Meinung bin, dass die von mir gefundenen Probleme erhebliche Anstrengungen erfordern, erstelle ich einfach ein Ticket im Issue-Tracker mit dem Titel "Probleme untersuchen und beheben, auf die in der Codeüberprüfung <Überprüfungs-ID / URL> hingewiesen wird" und fahre fort durch Code, der Kommentare in freier Form macht. Ich verschwende keine Zeit mit Dingen, die pro Ticket erledigt werden, das ich geöffnet habe - weil sie in die Teamarbeitswarteschlange eingehen und priorisiert und routinemäßig behandelt werden.
   
  Auf diese Weise stelle ich sicher, dass 1) der Überprüfungsprozess so einfach wie möglich bleibt und 2) entdeckte Probleme nicht vergessen werden.

Es sieht so aus, als ob Sie nach Codestriker suchen . Es verfügt über eine Funktion zum Klassifizieren von Problemen nach Modus, Stufe und Typ.

Ist diese Kategorisierung für die Teams, die regelmäßig die Codeüberprüfung üben, eine gängige Praxis?

Nun, da ich das obige Tool benutze, bin ich dazu gezwungen. Das Problem, das ich dabei sehe, ist, dass es manchmal schwierig sein kann, alles in Ordnung zu bringen, weil es so viele Möglichkeiten gibt.

Die Klassifizierung von Problemen bietet wenig Wert für die Codeüberprüfung, da der Kommentar wichtiger ist. Da ich gezwungen bin, für jeden Kommentar 4 Felder im obigen Tool festzulegen, wurde es tatsächlich umständlich.

Wenn Sie später die Zusammenfassung der Bewertungen überprüfen, können Sie feststellen, dass die Verteilung eine eindeutige Verteilung zu haben scheint. Menschen neigen dazu, nicht mehrmals denselben Fehler zu machen.

Wir codieren die Überprüfung anhand einer einfachen Haupt- / Neben- und Grundursache (Anforderungen / Design / Implementierung). Wenn Sie die Anzahl der Elemente niedrig halten, erhalten Sie brauchbare Metriken, ohne dass Sie zu feinkörnig werden müssen. Es gibt selten eine Diskussion über eine Kategorie, da zu wenige zur Auswahl stehen.

Unser Review-Tracking ist uralt, hat sich aber über 20 Jahre weiterentwickelt. Das Werkzeug der "Wahl" ist Excel, daher ist es bereits lästig. Wir wechseln zu Gerrit, um weniger formale Codeüberprüfungen durchzuführen (Außerhalb von "The Process", da es den Anforderungen der Entwickler besser entspricht - mit Gerrit schlagen wir einfach den Kommentar ein und fahren fort. Nicht die Erfassung von "Metriken".

Ich persönlich denke, dass Metriken überbewertet sind - bei der Überprüfung geht es um Codeverbesserung, und fragen Sie jeden, der an einigen teilgenommen hat, er wird Ihnen die gleichen Dinge sagen, die die Metriken tun werden. Mit den Metriken kann jedoch nicht argumentiert werden. Sie können bei Bedarf auch sinnvoll manipuliert werden, und das Management ist nicht klüger (ich bin manchmal ein bisschen zynisch).