Ist eine Codeüberprüfung, bei der nur Codekommentare verwendet werden, eine gute Idee?

Voraussetzungen

• Team nutzt DVCS
• IDE unterstützt das Parsen von Kommentaren (wie TODO usw.)
• Tools wie CodeCollaborator sind teuer fürs Budget
• Tools wie gerrit sind zu komplex für die Installation oder nicht verwendbar

Arbeitsablauf

• Der Autor veröffentlicht irgendwo auf der zentralen Repo-Zweigstelle
• Prüfer holen es und starten die Überprüfung

• Im Falle einer Frage / eines Problems können Sie einen Kommentar mit einem speziellen Label wie "REV" erstellen. Ein solches Etikett DARF NICHT im Produktionscode enthalten sein - nur in der Überprüfungsphase:

  $somevar = 123;
  // REV Why do echo this here?
  echo $somevar;
  

• Wenn der Rezensent mit dem Posten von Kommentaren fertig ist, wird nur die dumme Meldung "Kommentare" ausgegeben und zurückgeschoben

• Der Autor zieht den Feature-Zweig zurück und beantwortet Kommentare auf ähnliche Weise oder verbessert den Code und schiebt ihn zurück
• Wenn "REV" -Kommentare weg sind, können wir annehmen, dass diese Überprüfung erfolgreich abgeschlossen wurde.
• Der Autor baut den Feature-Zweig interaktiv neu auf, drückt ihn zusammen, um diese "Kommentar" -Eingaben zu entfernen, und ist nun bereit, das Feature zusammenzuführen, um eine Aktion zu entwickeln oder durchzuführen, die nach erfolgreicher interner Überprüfung normalerweise erfolgen könnte

IDE-Unterstützung

Ich weiß, dass benutzerdefinierte Kommentar-Tags in Eclipse & Netbeans möglich sind. Klar sollte es auch in der blablaStorm Familie sein.

Fragen

1. Halten Sie diese Methodik für sinnvoll?
2. Kennst du etwas ähnliches?
3. Was kann daran verbessert werden?

Die Idee ist eigentlich sehr schön. Im Gegensatz zu herkömmlichen Workflows wird die Überprüfung direkt im Code gespeichert. Technisch gesehen benötigen Sie also nur einen Texteditor , um diesen Workflow zu verwenden. Die Unterstützung in der IDE ist auch nett, insbesondere die Möglichkeit, die Liste der Bewertungen unten anzuzeigen.

Es gibt noch ein paar Nachteile:

• Es funktioniert gut für sehr kleine Teams, aber größere Teams müssen nachverfolgen, was wann von wem mit welchem ​​Ergebnis überprüft wurde. Obwohl Sie über diese Art der Verfolgung verfügen (die Versionskontrolle ermöglicht es, all das zu wissen), ist die Verwendung und Suche äußerst schwierig und erfordert häufig eine manuelle oder halbmanuelle Suche durch die Revisionen.

• Ich glaube nicht, dass der Prüfer genug Feedback vom Prüfer hat, um zu wissen, wie die geprüften Punkte tatsächlich angewendet wurden .

  Stellen Sie sich folgende Situation vor. Alice überprüft zum ersten Mal den Code von Eric. Sie bemerkt, dass Eric, ein junger Entwickler, die Syntax verwendet hat, die in der tatsächlich verwendeten Programmiersprache nicht die aussagekräftigste ist.

  Alice schlägt eine alternative Syntax vor und sendet den Code an Eric zurück. Er schreibt den Code mit dieser alternativen Syntax neu, die er für richtig hält, und entfernt den entsprechenden // BLAKommentar.

  In der nächsten Woche erhält sie den Code für die zweite Überprüfung. Würde sie sich tatsächlich daran erinnern können, dass sie diese Bemerkung bei ihrer ersten Überprüfung gemacht hatte, um zu sehen, wie Eric sie löste?

  In einem formaleren Überprüfungsprozess konnte Alice sofort feststellen, dass sie eine Bemerkung machte und den Unterschied des relevanten Codes sah, um festzustellen, dass Eric die Syntax, von der sie ihm erzählte, falsch verstanden hatte.

• Menschen sind immer noch Menschen. Ich bin mir ziemlich sicher, dass einige dieser Kommentare im Produktionscode landen und andere als Müll bleiben, obwohl sie völlig veraltet sind .

  Das gleiche Problem besteht natürlich auch bei anderen Kommentaren. Beispielsweise befinden sich viele TODO-Kommentare in der Produktion (einschließlich des nützlichsten: "TODO: Kommentieren Sie den folgenden Code.") und viele Kommentare, die nicht aktualisiert wurden, als der entsprechende Code vorhanden war.

  Beispielsweise kann der ursprüngliche Autor des Codes oder der Rezensent den Code verlassen, und der neue Entwickler würde nicht verstehen, was in der Rezension steht, so dass der Kommentar für immer erhalten bleibt und darauf wartet, dass jemand zu mutig wäre, ihn zu vernichten oder tatsächlich zu verstehen, was er tut es sagt.

• Dies ersetzt keine persönliche Überprüfung (das gleiche Problem tritt jedoch auch bei anderen formellen Überprüfungen auf, die nicht von Angesicht zu Angesicht durchgeführt werden).

  Insbesondere, wenn für die ursprüngliche Überprüfung eine Erklärung erforderlich ist, beginnen der Überprüfer und der Überprüfende eine Art Diskussion . Sie werden nicht nur mit großen BLA-Kommentaren konfrontiert sein, sondern diese Diskussionen werden auch das Protokoll der Versionskontrolle verschmutzen .

• Möglicherweise treten auch kleinere Probleme mit der Syntax auf (die auch für TODO-Kommentare vorhanden ist). Was ist zum Beispiel, wenn ein langer "// BLA" -Kommentar in mehreren Zeilen erscheint und jemand beschließt, ihn so zu schreiben:

  // BLA This is a very long comment which is way beyond 80 characters, so it actually
  // fills more then one line. Since the next lines start with slashes, but not "BLA"
  // keyword, the IDE may not be able to show those lines, and display only the first one.
  

• Und zum Schluss als kleine und sehr persönliche Anmerkung: Wählen Sie nicht "BLA" als Schlüsselwort. Es klingt hässlich. ;)

Ich würde die Kommentare im Code mit einem Begleitdokument ergänzen. Dies würde die Ergebnisse zusammenfassen und weiterleben, nachdem die Kommentare entfernt wurden. Die Vorteile davon wären:

• Kompaktheit. Wenn die Person routinemäßig nicht überprüft, ob ein übergebener Zeiger nicht null ist (oder ein anderer häufiger Anfängerfehler in der von Ihnen verwendeten Sprache), kann der Prüfer Dutzende von REV-Kommentaren zu diesem Zweck hinterlassen und im Dokument Folgendes angeben: Ich habe 37 Stellen gefunden, an denen Zeiger nicht zuerst überprüft wurden ", ohne sie alle aufzulisten
• Platz für Lob. Ein Kommentar wie dieser REV this is a nice designscheint seltsam zu sein, aber meine Code-Überprüfungen enthalten häufig sowohl Genehmigungen als auch Korrekturen
• Interaktivität. Ihr Beispielkommentar lautet "Warum haben Sie das getan?" und es ist ein sehr typisches. Ein Ansatz nur mit Kommentaren unterstützt keine Konversation. Die Person kann ihren Code ändern und den Kommentar löschen oder den Kommentar löschen. Aber "warum hast du das getan?" und "bitte ändere das, es ist falsch" sind nicht dasselbe.
• ein Protokoll führen. Ein späterer Prüfer kann überprüfen, ob der Code tatsächlich geändert oder die Kommentare nur entfernt wurden. Sie können auch überprüfen, ob die Kommentare "berücksichtigt" wurden und der Entwickler in einer nachfolgenden Überprüfung nicht mehr dieselben Fehler macht.

Ich würde auch ein Arbeitselement verwenden, um die Überprüfung durchzuführen und auf die Überprüfung zu antworten, und die Eincheckvorgänge damit verknüpfen. Das macht es einfach, die Kommentare in einem alten Änderungssatz zu finden und zu sehen, wie jeder in einem anderen Änderungssatz behandelt wurde.

Andere haben über die Grenzen dieses Ansatzes gesprochen. Sie haben erwähnt, dass Tools wie gerrit schwer zu installieren sind - ich schlage vor, Sie werfen einen Blick auf phabricator (http://www.phabricator.com). Dies ist das Codeüberprüfungssystem, das Facebook seit Jahren verwendet und das vor kurzem als Open-Source-Lösung angeboten wurde. Es ist nicht schwer zu installieren, verfügt über hervorragende Arbeitsabläufe und behebt alle in den anderen Kommentaren genannten Probleme.