Nach einigen gravierenden Qualitätsproblemen im letzten Jahr hat mein Unternehmen kürzlich Code-Reviews eingeführt. Der Codeüberprüfungsprozess wurde schnell ohne Richtlinien oder Checklisten eingeführt.

Ein anderer Entwickler und ich haben uns vorgenommen, alle an den Systemen vorgenommenen Änderungen zu überprüfen, bevor sie in den Stamm integriert werden.

Wir wurden auch als "Technical Lead" ausgewählt. Dies bedeutet, dass wir für die Codequalität verantwortlich sind, aber keine Berechtigung haben, Änderungen im Prozess vorzunehmen, Entwickler neu zuzuweisen oder Projekte zurückzuhalten.

Technisch gesehen können wir die Fusion ablehnen und der Entwicklung zurückgeben. In Wirklichkeit endet dies fast immer damit, dass unser Chef verlangt, dass es pünktlich verschickt wird.

Unser Manager ist ein MBA, der sich hauptsächlich mit der Erstellung eines Zeitplans für anstehende Projekte befasst. Während er es versucht, hat er fast keine Ahnung, was unsere Software aus geschäftlicher Sicht tut, und es fällt ihm schwer, selbst die grundlegendsten Kundenanforderungen ohne Erklärung eines Entwicklers zu verstehen.

Derzeit wird die Entwicklung in Entwicklungszweigen in SVN durchgeführt. Nachdem der Entwickler glaubt, dass er bereit ist, weist er das Ticket in unserem Ticketsystem unserem Manager zu. Der Manager weist es uns dann zu.

Die Code-Reviews haben zu einigen Spannungen in unserem Team geführt. Besonders einige der älteren Mitglieder hinterfragen die Änderungen (zB "Wir haben es immer so gemacht" oder "Warum sollte die Methode einen vernünftigen Namen haben, ich weiß was sie tut?").

Nach den ersten Wochen begann meine Kollegin die Dinge laufen zu lassen, um keine Probleme mit den Mitarbeitern zu verursachen (sie sagte mir selbst, dass sie nach einem Fehlerbericht, der von einem Kunden eingereicht wurde, von dem Fehler wusste, aber befürchtete, dass die Entwickler wäre sauer auf sie, wenn er darauf hinweist).

Andererseits bin ich jetzt dafür bekannt, ein Esel zu sein, um auf Probleme mit dem festgeschriebenen Code hinzuweisen.

Ich denke nicht, dass meine Standards zu hoch sind.

Meine Checkliste ist im Moment:

• Der Code wird kompiliert.
• Es gibt mindestens eine Möglichkeit, wie der Code funktioniert.
• Der Code funktioniert in den meisten normalen Fällen.
• Der Code funktioniert mit den meisten Randfällen.
• Der Code löst eine angemessene Ausnahme aus, wenn die eingegebenen Daten ungültig sind.

Aber ich übernehme die Verantwortung für die Art und Weise, wie ich Feedback gebe. Ich gebe bereits umsetzbare Punkte an, um zu erklären, warum etwas geändert werden sollte, und manchmal auch nur zu fragen, warum etwas auf eine bestimmte Weise implementiert wurde. Wenn ich es für schlecht halte, weise ich darauf hin, dass ich es auf andere Weise entwickelt hätte.

Was mir fehlt, ist die Fähigkeit, etwas zu finden, das ich als "gut" bezeichnen kann. Ich habe gelesen, dass man versuchen sollte, schlechte Nachrichten in gute Nachrichten zu stecken.

Aber es fällt mir schwer, etwas Gutes zu finden. "Hey, diesmal hast du wirklich alles begangen, was du getan hast" ist herablassender als nett oder hilfsbereit.

Beispiel Code Review

Hallo Joe,

Ich habe einige Fragen zu Ihren Änderungen in der Library \ ACME \ ExtractOrderMail-Klasse.

Ich habe nicht verstanden, warum Sie "TempFilesToDelete" als statisch markiert haben. Im Moment würde ein zweiter Aufruf von "GetMails" eine Ausnahme auslösen, da Sie Dateien hinzufügen, diese aber niemals entfernen, nachdem Sie sie gelöscht haben. Ich weiß, dass die Funktion nur einmal pro Lauf aufgerufen wird, aber in Zukunft könnte sich dies ändern. Könnten Sie es einfach zu einer Instanzvariablen machen, dann könnten wir mehrere Objekte parallel haben.

... (Einige andere Punkte, die nicht funktionieren)

Kleinere Punkte:

• Warum nimmt "GetErrorMailBody" eine Ausnahme als Parameter? Habe ich etwas verpasst? Sie lösen die Ausnahme nicht aus, sondern geben sie einfach weiter und rufen "ToString" auf. Warum ist das so?
• SaveAndSend Ist kein guter Name für die Methode. Diese Methode sendet Fehler-Mails, wenn die Verarbeitung einer Mail fehlgeschlagen ist. Könnten Sie es in "SendErrorMail" oder etwas Ähnliches umbenennen?
• Bitte kommentieren Sie alten Code nicht nur, sondern löschen Sie ihn sofort. Wir haben es immer noch in Subversion.

Wie finde ich positive Dinge in einer Codeüberprüfung?

Nach einigen gravierenden Qualitätsproblemen im letzten Jahr hat mein Unternehmen kürzlich Code-Reviews eingeführt.

Großartig, Sie haben eine echte Chance, Wert für Ihr Unternehmen zu schaffen.

Nach den ersten Wochen begann meine Kollegin die Dinge laufen zu lassen, um keine Probleme mit den Mitarbeitern zu verursachen (sie sagte mir selbst, nachdem ein Kunde einen Bugreport eingereicht hatte, wusste sie von dem Bug, befürchtete aber, dass der Entwickler wäre sauer auf sie, wenn sie darauf hingewiesen hätte).

Ihre Mitarbeiter sollten keine Codeüberprüfung durchführen, wenn sie nicht damit umgehen kann, Entwicklern mitzuteilen, was mit ihrem Code nicht stimmt. Es ist Ihre Aufgabe , Probleme zu finden und zu beheben, bevor sie Kunden betreffen.

Ebenso bittet ein Entwickler, der Mitarbeiter einschüchtert, entlassen zu werden. Ich habe mich nach einer Codeüberprüfung eingeschüchtert gefühlt - ich habe es meinem Chef gesagt, und es wurde gehandhabt. Außerdem mag ich meinen Job, also habe ich das Feedback positiv und negativ aufrechterhalten. Als Rezensent liegt das an mir, nicht an irgendjemand anderem.

Andererseits bin ich jetzt dafür bekannt, ein Esel zu sein, um auf Probleme mit dem festgeschriebenen Code hinzuweisen.

Nun, das ist bedauerlich, du sagst, du bist taktvoll. Sie finden mehr zu loben, wenn Sie mehr zu suchen haben.

Kritisieren Sie den Code, nicht den Autor

Sie geben ein Beispiel:

Ich habe einige Fragen zu Ihren Änderungen in

Vermeiden Sie es, die Wörter "Sie" und "Ihr" zu verwenden, sagen Sie "die" Änderungen stattdessen.

Habe ich etwas verpasst? [...] Warum ist das so?

Fügen Sie Ihren Kritiken keine rhetorischen Schnörkel hinzu. Mach auch keine Witze. Es gibt eine Regel, die ich gehört habe: "Wenn du dich gut fühlst zu sagen, sag es nicht, es ist nicht gut."

Vielleicht polieren Sie Ihr eigenes Ego auf Kosten eines anderen. Behalten Sie es nur die Fakten.

Erhöhen Sie die Messlatte, indem Sie positives Feedback geben

Es erhöht die Messlatte, um Ihre Kollegen zu loben, wenn sie höhere Standards erfüllen. Das heißt also die Frage,

Wie finde ich positive Dinge in einer Codeüberprüfung?

ist eine gute und es lohnt sich, sie anzusprechen.

Sie können darauf hinweisen, wo der Code Ideale höherer Codierungspraktiken erfüllt.

Achten Sie darauf, dass sie bewährten Methoden folgen und die Messlatte weiterhin höher legen. Nachdem die leichteren Ideale von allen erwartet werden, sollten Sie aufhören, sie zu loben, und nach noch besseren Codierungsmethoden suchen, um sie zu loben.

Sprachspezifische Best Practices

Wenn die Sprache Dokumentation in Code, Namespaces, objektorientierten oder funktionalen Programmierfunktionen unterstützt, können Sie diese aufrufen und dem Autor dazu gratulieren, dass er sie gegebenenfalls verwendet. Diese Themen fallen normalerweise unter die Stilrichtlinien:

• Entspricht es den firmeninternen Standards für Sprachrichtlinien?
• Entspricht es den maßgeblichsten Stilrichtlinien für die Sprache (die wahrscheinlich strenger ist als die firmeninterne - und damit immer noch dem firmeninternen Stil entspricht)?

Allgemeine Best Practices

Unter verschiedenen Paradigmen konnte man Punkte finden, die man für generische Codierungsprinzipien loben sollte. Haben sie zum Beispiel gute Unittests? Decken die Unittests den größten Teil des Codes ab?

Suchen:

• Unit-Tests, die nur die Funktionalität des Subjekts testen - verspotten teure Funktionen, die nicht getestet werden sollen.
• Hohe Codeabdeckung mit vollständigem Testen der APIs und semantisch öffentlicher Funktionalität.
• Akzeptanztests und Rauchtests, die die End-to-End-Funktionalität testen, einschließlich Funktionen, die für Komponententests nachgeahmt werden.
• Gute Benennung, kanonische Datenpunkte, sodass der Code trocken ist (Don't Repeat Yourself), keine magischen Zeichenfolgen oder Zahlen.
• Variable Benennung so gut gemacht, dass Kommentare weitgehend überflüssig sind.
• Aufräumarbeiten, objektive Verbesserungen (ohne Kompromisse) und geeignete Überarbeitungen, die Codezeilen und technische Schulden reduzieren, ohne dass der Code den ursprünglichen Autoren vollständig fremd ist.

Funktionale Programmierung

Wenn die Sprache funktional ist oder das Funktionsparadigma unterstützt, suchen Sie nach den folgenden Idealen:

• Vermeidung globaler und globaler Staaten
• mit Verschlüssen und Teilfunktionen
• kleine Funktionen mit lesbaren, korrekten und beschreibenden Namen
• einzelne Austrittspunkte, wodurch die Anzahl der Argumente minimiert wird

Objektorientierte Programmierung (OOP)

Wenn die Sprache OOP unterstützt, können Sie die angemessene Verwendung dieser Funktionen loben:

• Kapselung - Bietet eine klar definierte und kleine öffentliche Schnittstelle und verbirgt die Details.
• Vererbungscode wird angemessen wiederverwendet, möglicherweise durch Mixins.
• Polymorphismus - Schnittstellen sind definierte, möglicherweise abstrakte Basisklassen, Funktionen, die zur Unterstützung des parametrischen Polymorphismus geschrieben wurden.

Unter OOP gibt es auch SOLID- Prinzipien (möglicherweise Redundanz gegenüber OOP-Funktionen):

• Einzelverantwortung - Jedes Objekt hat einen Stakeholder / Eigentümer
• open / closed - Ändert nicht die Schnittstelle festgelegter Objekte
• Liskov-Substitution - Für Instanzen von Eltern können Unterklassen eingesetzt werden
• Schnittstellentrennung - Schnittstellen, die von der Zusammensetzung bereitgestellt werden, möglicherweise Mixins
• Abhängigkeitsinversion - Schnittstellen definiert - Polymorphismus ...

Unix - Programmierung Prinzipien :

Unix-Prinzipien sind Modularität, Klarheit, Komposition, Trennung, Einfachheit, Sparsamkeit, Transparenz, Robustheit, Repräsentation, geringste Überraschung, Stille, Reparatur, Wirtschaftlichkeit, Generierung, Optimierung, Vielfalt und Erweiterbarkeit.

Im Allgemeinen können diese Prinzipien unter vielen Paradigmen angewendet werden.

Ihre Kriterien

Diese sind viel zu trivial - ich würde mich herablassen, wenn ich dafür gelobt würde:

• Der Code wird kompiliert.
• Es gibt mindestens eine Möglichkeit, wie der Code funktioniert.
• Der Code funktioniert in den meisten normalen Fällen.

Auf der anderen Seite sind diese ziemlich hoch gelobt, wenn man bedenkt, womit Sie es zu tun scheinen, und ich würde nicht zögern, die Entwickler dafür zu loben:

• Der Code funktioniert mit den meisten Randfällen.
• Der Code löst eine angemessene Ausnahme aus, wenn die eingegebenen Daten ungültig sind.

Regeln für die Codeüberprüfung aufschreiben?

Theoretisch ist das eine großartige Idee. Obwohl ich normalerweise keinen Code wegen falscher Benennung ablehne, habe ich gesehen, dass die Benennung so schlecht ist, dass ich den Code mit Anweisungen zur Fehlerbehebung ablehne. Sie müssen in der Lage sein, den Code aus irgendeinem Grund abzulehnen.

Die einzige Regel, die ich mir vorstellen kann, um Code abzulehnen, ist, dass nichts so ungeheuerlich ist, dass ich ihn aus der Produktion verbannen würde. Ein wirklich schlechter Name ist etwas, das ich gerne aus der Produktion verbannen würde - aber das kann man nicht zur Regel machen.

Fazit

Sie können Best Practices loben, die unter verschiedenen Gesichtspunkten befolgt werden, und wahrscheinlich unter allen, wenn die Sprache sie unterstützt.

Suchen Sie sich nur dann etwas Gutes aus, wenn es sich um ein solides Beispiel handelt, das in direktem Zusammenhang mit dem eigentlichen Thema steht.

Ich werde es nicht beschönigen - nach den Geräuschen haben Sie es mit mindestens einer Person zu tun, die unsicher in Bezug auf ihre Fähigkeiten ist und mit unreifen Herausforderungen in Bezug auf ihre Arbeit umgeht. Sie sind wahrscheinlich auch schlecht in ihrer Arbeit - ein guter Entwickler sollte immer bereit sein, sich selbst zu reflektieren, konstruktive Kritik zu üben und offen dafür zu sein, ihre Verhaltensweisen zu ändern.

Jetzt, wo das in der Luft liegt, können wir über dich reden. Unabhängig davon, ob Sie denken, dass Sie vernünftig sind, müssen Sie besonders vorsichtig mit solchen Leuten sein, um den Ball ins Rollen zu bringen. Ich habe herausgefunden, dass der beste Weg, mit diesen Leuten umzugehen, darin besteht, sehr vorsichtig damit umzugehen, wie Sie Dinge ausdrücken.

Stellen Sie sicher, dass Sie den Code und nicht den Autor beschuldigen . Konzentrieren Sie sich auf das eine Problem und nicht auf den Poo-Berg, der Ihre Codebasis ist, den sie möglicherweise maßgeblich mitgestaltet haben und der als weiterer persönlicher Angriff angesehen werden würde. Wählen Sie zunächst Ihre Schlachten aus, und konzentrieren Sie sich auf kritische Themen, die sich für Ihre Benutzer manifestieren, damit Sie keine Kritik an der Person auslösen, die sie dazu bringt, alles, was Sie sagen, zu verwerfen.

Körpersprache und Ton sind wichtig, wenn Sie persönlich mit ihnen sprechen, klar sein, was Sie sagen, und sicherstellen, dass Sie nicht mit ihnen reden oder ihre technischen Fähigkeiten ablehnen. Sie werden höchstwahrscheinlich von Anfang an in der Defensive sein, also müssen Sie ihre Sorgen klären, anstatt sie zu bestätigen. Sie müssen sich dieser Konversation bewusst sein, ohne zu offensichtlich zu sein, damit sie unbewusst glauben, Sie seien auf ihrer Seite, und hoffentlich akzeptieren sie, dass sie die Änderungen vornehmen müssen, auf die aufmerksam gemacht wurde.

Wenn dies nicht funktioniert, können Sie etwas aggressiver werden. Wenn das Produkt von einem Konferenzraum aus ausgeführt werden kann, rufen Sie es während der Codeüberprüfung auf dem Projektor auf und zeigen Sie den Fehler aus erster Hand. Es ist besser, wenn ein Manager direkt vor Ort ist, damit die Person nicht zurücktreten kann. Dies soll sie nicht beschämen, sondern dazu zwingen, zu akzeptieren, dass das Problem für den Benutzer real ist und dass es behoben werden muss, anstatt nur eine Beschwerde, die Sie mit ihrem Code haben.

Wenn Sie irgendwann nicht mehr weiterkommen, es satt haben, die Person wie eine Kindergärtnerin zu behandeln, und die Geschäftsleitung sich des Problems überhaupt nicht bewusst ist, wirkt sich dies entweder schlecht auf Ihre Leistung als Code-Prüfer aus oder Sie kümmern sich um das Wohlergehen Ihrer Mitarbeiter Firma und / oder Produkt, müssen Sie anfangen, mit Ihrem Chef über ihr Verhalten zu sprechen. Seien Sie so spezifisch und unpersönlich wie möglich - stellen Sie ein Geschäftsmodell auf, bei dem Produktivität und Qualität leiden.

Code-Reviews können giftig sein, Zeit verschwenden und lebensraubende Nerd-Kriege auslösen. Schauen Sie sich die Meinungsverschiedenheiten bei Dingen wie sauberem Code gegenüber Kommentaren, Namenskonventionen, Unit- und Integrationstests, Eincheckstrategien, RESTfulness usw. an.

Die einzige Möglichkeit, dies zu vermeiden, besteht darin , die Regeln für das Bestehen einer Codeüberprüfung aufzuschreiben.

Dann ist es keine Person, die den Check-in nicht bestanden oder genehmigt hat. Sie überprüfen lediglich, ob die Regeln eingehalten wurden.

Und weil sie im Voraus geschrieben wurden, können Sie beim Schreiben Ihres Codes die Regeln befolgen und müssen später weder Ihre Argumente noch Argumente erläutern.

Wenn Ihnen die Regeln nicht gefallen, können Sie sie in einem Meeting ändern.

Ich würde Ihr Feedback nicht beschönigen, da es als bevormundend angesehen werden kann.

Meiner Meinung nach ist es die beste Vorgehensweise, sich immer auf den Code und niemals auf den Autor zu konzentrieren.

Es ist ein Code Bewertung, kein Entwickler Bewertung, so:

• "Diese while-Schleife wird möglicherweise nie beendet", nicht "Ihre Schleife wird möglicherweise nie beendet"
• "Für Szenario X ist ein Testfall erforderlich", nicht "Sie haben den Test nicht geschrieben, um dieses Szenario abzudecken".

Es ist auch sehr wichtig, die gleiche Regel anzuwenden, wenn Sie mit anderen über die Überprüfung sprechen:

• "Anne, was denkst du über diesen Code?", Nicht "Anne, was denkst du über Jons Code?"

Die Codeüberprüfung ist nicht die Zeit für eine Leistungsüberprüfung - sie sollte separat durchgeführt werden.

Ich bin überrascht, dass es in keiner Antwort erwähnt wurde, und vielleicht ist meine Erfahrung mit Code-Reviews die ungewöhnliche, aber:

Warum überprüfen Sie die gesamte Zusammenführungsanforderung in einer einzelnen Nachricht?

Meine Erfahrungen mit Code Reviews stammen von GitLab. Ich habe mir immer vorgestellt, dass andere Tools zur Codeüberprüfung ähnlich funktionieren würden.

Wenn ich eine Codeüberprüfung gebe, kommentiere ich bestimmte, einzelne Zeilen des Unterschieds. Es ist äußerst unwahrscheinlich, dass dies als persönliche Kritik aufgenommen wird, da es sich um einen Kommentar zum Code handelt - und tatsächlich als Kommentar zum Code angezeigt wird, der direkt unter dem Code angezeigt wird, zu dem er ein Kommentar ist.

Ich kann auch die gesamte Zusammenführungsanforderung kommentieren und oft tun. Bestimmte Punkte können jedoch auf bestimmten Linien des Diffs hervorgehoben werden. (Wenn ein neues Commit hinzugefügt wird, sodass sich das Diff ändert, werden die Kommentare zum "veralteten Diff" standardmäßig ausgeblendet.)

Mit diesem Workflow werden Codeüberprüfungen viel modularer und weniger zusammenhängend. Eine Zeile aus einer Codeüberprüfung kann einfach sagen:

Netter Ansatz, dies in eine dedizierte Funktion zu packen!

Oder es könnte sagen,

Dieser Objektname entspricht nicht wirklich dem Zweck des Objekts. Vielleicht könnten wir stattdessen einen Namen wie 'XYZ' verwenden?

Oder wenn es große Probleme mit einem Abschnitt gibt, könnte ich schreiben:

Ich sehe, dass dieser Code funktioniert (und ich verstehe, warum er funktioniert), aber es bedarf einer gezielten Untersuchung, um ihn zu verstehen. Könnten Sie es bitte in separate Funktionen umgestalten, damit es in Zukunft einfacher zu warten ist?

(Beispiel: Funktion ABC macht hier tatsächlich drei Dinge: das Foo bazzing, das Boz sperren und das Zorf ausfransen. Das können alles separate Funktionen sein.)

Nachdem ich alles oben Genannte geschrieben habe, kann ich einen zusammenfassenden Kommentar zur gesamten Zusammenführungsanforderung abgeben, etwa:

Dies ist eine großartige neue Funktion, die nach dem Zusammenführen sehr nützlich sein wird. Können Sie bitte die Benennung der Funktion bereinigen und das Refactoring durchführen, das in den einzelnen Kommentaren erwähnt wurde, und mich dann informieren, um es noch einmal anzusehen? :)

Auch wenn es sich bei dem Zusammenführungswunsch um ein vollständiges Hundefrühstück handelt, können die einzelnen Kommentare jeweils einfach sein. Es wird einfach mehr von ihnen geben. Dann könnte der zusammenfassende Kommentar lauten:

Es tut mir leid, aber dieser Code entspricht nicht wirklich dem Schnupftabak. Es gibt sehr viele Randfälle (wie in einzelnen Kommentaren beschrieben), die falsch gehandhabt werden und in einem Fall zu einer schlechten Benutzererfahrung oder sogar zu einer Datenbeschädigung führen. (Siehe Kommentar zu Commit 438a95fb734.) Sogar einige normale Anwendungsfälle führen zu einer extrem schlechten Anwendungsleistung (Einzelheiten in den einzelnen Kommentaren zum Diff für somefile.c).

Um produktionsbereit zu sein, muss diese Funktion vollständig umgeschrieben werden, wobei mehr darauf geachtet werden muss, welche Annahmen an jedem Punkt des Ablaufs sicher getroffen werden können. (Hinweis: Keine Annahmen sind sicher, es sei denn, sie wurden überprüft.)

Ich schließe die Zusammenführungsanforderung bis zum vollständigen Umschreiben.

Zusammenfassung: Überprüfen Sie die technischen Aspekte des Codes als Kommentare zu einzelnen Codezeilen. Dann fasst diese Kommentare in einem Gesamt Kommentar auf der Zusammenführung Anfrage. Nicht persönlich werden - behandeln Sie nur die Fakten und Ihre Meinung zum Code , nicht zum Codierer. Und stützen Sie Ihre Meinung auf Fakten, genaue Beobachtung und Verständnis.

Ich bin wirklich überrascht, dass niemand das aufgegriffen hat, aber es stimmt etwas nicht mit der veröffentlichten Beispielbewertung.

Es gibt einfach keinen Grund, warum Sie sich direkt an Joe wenden sollten. Dass Joe seine Fehler behebt, geht Sie nichts an. Das macht jemand , ist deine Sache. Ihr Anliegen ist die Codequalität. Anstatt also Anfragen / Bestellungen / Forderungen zu schreiben (wenn ich Joe wäre, könnte ich dies einfach ablehnen, weil Sie dafür nicht legitim sind), bleiben Sie in Ihrer Rolle und schreiben Sie eine einfache anonyme ToDo-Liste.

Der Versuch, fair mit guten und schlechten Punkten umzugehen, ist nicht nur unmöglich, sondern liegt auch völlig außerhalb Ihrer Rolle.

Hier ein Beispiel für eine Neuformulierung mit Inhalten aus Ihrer Rezension:

• Bevor wir Änderungen in der Library \ ACME \ ExtractOrderMail-Klasse übernehmen, müssen wir einige Probleme beheben.
• Sofern ich nichts verpasst habe, sollte "TempFilesToDelete" nicht statisch sein.
• In Zukunft können wir die Funktion mehr als einmal pro Lauf aufrufen, deshalb brauchen wir (was hier zu tun ist).
• Ich muss verstehen, warum "GetErrorMailBody" eine Ausnahme als Parameter nimmt. (Und ich bin hier an der Grenze, weil du jetzt schon eine Schlussfolgerung haben solltest. )
• SaveAndSend sollte umbenannt werden, damit es besser zu seinem Verhalten passt, z. B. zu "SendErrorMail".
• Auskommentierter Code sollte aus Gründen der Lesbarkeit gelöscht werden. Wir verwenden Subversion für eventuelle Rollbacks.

Wenn Sie die Rezension so formulieren, egal wie sehr der Leser Sie persönlich hasst, kann er hier nur Notizen zu Verbesserungen sehen, die später von jemandem durchgeführt werden müssen. Wer ? Wann ? Niemanden interessierts. Was denn Warum ? DAS solltest du sagen.

Sie werden jetzt genau den Grund angehen, warum Code-Reviews die Spannung erhöhen, indem Sie den Faktor Mensch aus der Gleichung streichen.

Der springende Punkt bei der Codeüberprüfung ist das Auffinden von Problemen. Wenn es einen Fehler gibt, können Sie am besten einen fehlgeschlagenen Testfall schreiben.

Ihr Team (Manager) sollte mitteilen, dass das Produzieren von Fehlern Teil des Spiels ist, aber das Finden und Beheben von Fehlern wird die Arbeit aller retten .

Es kann hilfreich sein, regelmäßige Meetings (entweder Team oder Paar) abzuhalten und einige Probleme zu lösen. Der ursprüngliche Programmierer hat absichtlich keine Probleme eingeführt, und manchmal könnte er denken, dass es gut genug ist (und manchmal sogar). Aber dieses zweite Augenpaar bietet eine völlig neue Sicht und er kann viel lernen, wenn er sich die Probleme ansieht.

Es ist wirklich eine kulturelle Sache, und es braucht viel Vertrauen und Kommunikation. Und Zeit, um mit den Ergebnissen zu arbeiten.

Ich denke, das Positive wäre, einen Konsens über die Codierungsstandards zu erzielen, bevor Sie Überprüfungen durchführen. Andere kaufen eher etwas ein, wenn sie etwas eingegeben haben.

Fragen Sie bei Namenskonventionen andere, ob dies wichtig ist. Die meisten Entwickler werden sich vor allem unter Gleichaltrigen einig sein. Wer möchte die Person sein, die mit etwas nicht einverstanden sein möchte, das in der Programmierwelt so weit verbreitet ist? Wenn Sie den Prozess starten, indem Sie den Code einer Person auswählen und auf den Fehler hinweisen, wird diese Person sehr defensiv. Wenn Standards festgelegt sind, wird es Meinungsverschiedenheiten über die Interpretation oder das geben, was als "gut genug" angesehen wird, aber es geht Ihnen besser als jetzt.

Ein weiterer Teil dieses Prozesses besteht darin, zu bestimmen, wie Codeüberprüfungen mit Einwänden umgehen. Dies kann nicht zu einer endlosen Debatte werden. Irgendwann muss jemand die Entscheidung treffen. Vielleicht kann es einen Dritten geben, der der Richter ist, oder der Prüfer hat die volle Macht. Dinge, die erledigt werden müssen, sollten das Ziel sein.

Das letzte Stück dabei ist, alle wissen zu lassen, dass Code Reviews nicht Ihre Idee waren, sie bleiben werden, daher sollten sich alle Mühe geben, das Beste daraus zu machen. Wenn sich jeder machtlos fühlt, kann er möglicherweise Ihren Code überprüfen?

Es ist zu hoffen, dass ein messbares Ergebnis für das Management darin besteht, Fehler, Kundenbeschwerden, Verzögerungen usw. zu begrenzen. Andernfalls hat gerade jemand das Schlagwort "Codeüberprüfung" gehört und herausgefunden, dass Wunder ohne viel Zeit und Energie geschehen und Mühe in diese.

Dies mag hart sein, aber machen Sie sich keine Sorgen über ein gutes Feedback, wenn es nichts Gutes zum Messen gibt.

Wenn Ihre Entwickler jedoch in Zukunft beginnen, ihren Code zu verbessern, sollten Sie ihnen ein gutes Feedback geben. Sie möchten auf die Verbesserungen im Code und auf die Vorteile für das gesamte Team hinweisen. Wenn Sie anfangen, eine Verbesserung zu bemerken, werden sie es auch, und die Dinge sollten sich langsam herumsprechen.

Etwas anderes; Es kann eine defensive Atmosphäre geben, weil sie das Gefühl haben, kein Mitspracherecht zu haben. Warum lassen Sie sie nicht den Code des jeweils anderen überprüfen? Stellen Sie ihnen spezifische Fragen und versuchen Sie, sie einzubeziehen. Es sollte nicht du gegen sie sein; Es sollte eine Teamleistung sein.

1. Was würden Sie an diesem Code ändern, wenn Sie Zeit hätten?
2. Wie würden Sie diesen Bereich der Codebasis verbessern?

Fragen Sie das jetzt, und fragen Sie das in sechs Monaten. Hier gibt es eine Lernerfahrung.

Der wichtigste Punkt - loben Sie nicht den Code, für den keine Garantie besteht, sondern erkennen Sie den Aufwand und die Verbesserung an. Versuchen Sie, Bewertungen zu einer Teamübung zu machen, anstatt zu einer gegnerischen.

Qualität ohne Spannung

Sie haben gefragt, wie Sie positive Aussagen zu Code finden können. Ihre eigentliche Frage ist jedoch, wie Sie „Spannungen innerhalb Ihres Teams“ vermeiden und gleichzeitig „schwerwiegende Qualitätsprobleme“ angehen können.

Der alte Trick, „schlechte Nachrichten in gute Nachrichten“ einzuschieben, könnte fehlschlagen. Entwickler werden es wahrscheinlich als das sehen, was es ist: eine Erfindung.

Ihre Organisationen Top-Down-Probleme

Ihre Chefs haben Sie mit der Qualitätssicherung beauftragt. Sie haben eine Liste mit Kriterien für die Codequalität erstellt. Nun möchten Sie Ideen für eine positive Verstärkung für Ihr Team.

Warum fragen Sie uns, was Sie tun müssen, um Ihr Team glücklich zu machen? Haben Sie überlegt, Ihr Team zu fragen?

Es hört sich so an, als würden Sie Ihr Bestes geben, um nett zu sein. Das Problem ist nicht, wie Sie Ihre Nachricht übermitteln. Das Problem ist, dass die Kommunikation einseitig war.

Aufbau einer Kultur der Qualität

Eine Kultur der Qualität muss gepflegt werden, um von Grund auf zu wachsen.

Lassen Sie Ihren Chef wissen, dass Sie besorgt sind, dass sich die Qualität nicht schnell genug verbessert, und Sie möchten versuchen, einige Ratschläge von The Harvard Business Review anzuwenden .

Treffen Sie sich mit Ihrem Team. Modellieren Sie das Verhalten, das Sie in Ihrem Team sehen möchten: Demut, Respekt und die Verpflichtung zur Verbesserung.

Sagen Sie so etwas wie: „Wie Sie wissen, waren [Mitarbeiter] und ich damit beauftragt, die Codequalität sicherzustellen, um die Art von Produktionsproblemen zu vermeiden, unter denen wir in letzter Zeit gelitten haben. Ich persönlich glaube nicht, dass ich dieses Problem gelöst habe. Ich glaube, mein größter Fehler war es, Sie am Anfang nicht alle mehr einzubeziehen. [Mitarbeiter] und ich sind immer noch verantwortlich für die Qualität des Codes an das Management, sondern geht nach vorn, wir alle zusammen in der Qualität Anstrengung sind.“

Lassen Sie sich vom Team Anregungen zur Codequalität geben. (Ein Whiteboard würde helfen.) Stellen Sie sicher, dass Ihre Anforderungen bis zum Ende erfüllt sind. Bieten Sie Ihre Erkenntnisse auf respektvolle Weise an und stellen Sie gegebenenfalls Fragen. Seien Sie bereit, überrascht zu sein, was Ihrem Team wichtig ist. Seien Sie flexibel, ohne die geschäftlichen Anforderungen zu beeinträchtigen.

(Wenn Sie alten Code haben, der Ihnen peinlich ist, können Sie ihn ausprobieren und alle dazu ermutigen, offen zu sein. Lassen Sie die Leute am Ende wissen, dass Sie ihn geschrieben haben. Modellieren Sie die ausgereifte Reaktion, auf die Sie hoffen, wenn andere Kritik am Bau erhalten. )

Kollaborative Code-Überprüfungen

Ich habe nicht an einem Ort gearbeitet, wie Sie es beschreiben, an dem einige leitende Programmierer den gesamten Code überprüfen und Korrekturen vornehmen. Kein Wunder, dass Leute antworten, als wären Sie ein Lehrer, der rote Markierungen auf ihren Papieren macht.

Wenn Sie die Idee an das Management verkaufen können, beginnen Sie mit Peer-Code-Überprüfungen . Dies geschieht am besten in kleinen Gruppen, einschließlich Ihnen oder dem anderen verantwortlichen Entwickler. Stellen Sie sicher, dass jeder mit Respekt behandelt wird.

Ein wichtiges Ziel des Peer-Review-Codes ist es, sicherzustellen, dass der Code von allen Mitgliedern des Teams verstanden wird. Stellen Sie sich ein Beispiel für unklare Funktionsnamen vor: Wenn Sie von einem jüngeren Entwickler hören, dass der Funktionsname verwirrend ist, kann dies leichter akzeptiert werden als eine andere „Korrektur“ des älteren Entwicklers.

Qualität ist eine Reise

Eine andere Sache, die zu tun ist, besteht darin, die Vorstellung zu verwerfen, dass eine Codeüberprüfung ein Bestehen / Nichtbestehen ist. Jeder sollte damit rechnen, nach einer Codeüberprüfung einige Änderungen vorzunehmen. (Und Ihre Aufgabe ist es, sicherzustellen, dass sie passieren.)

Aber wenn das nicht geht ...

Nehmen wir an, Sie machen einige Schritte, um eine Kultur der Qualität zu etablieren. Vielleicht holen Sie noch nicht alle an Bord. Wenn jemand immer noch nicht mit dem Qualitätsprogramm vertraut ist, besteht das Problem darin, dass er sich nicht in das Team einfügt, anstatt dass es ein Problem zwischen Ihnen beiden gibt. Wenn sie aus dem Team entlassen werden müssen, versteht der Rest des Teams die Gründe besser.

Entschuldigen Sie noch eine lange Antwort, aber ich glaube nicht, dass die anderen das menschliche Element dieses Problems vollständig angesprochen haben.

manchmal sogar nur gefragt, warum etwas auf eine bestimmte Weise implementiert wurde. Wenn ich es für schlecht halte, weise ich darauf hin, dass ich es auf andere Weise entwickelt hätte.

Das Problem mit "Warum haben Sie es so implementiert?" ist, dass Sie den Entwickler sofort in die Defensive bringen. Die Frage selbst kann je nach Kontext alles Mögliche bedeuten: Sind Sie zu dumm, um eine bessere Lösung zu finden? Ist das das Beste, was du tun kannst? Versuchen Sie, dieses Projekt zu ruinieren? Kümmerst du dich überhaupt um die Qualität deiner Arbeit? usw. Die Frage, warum der Code auf eine bestimmte Weise entwickelt wurde, ist nur konfrontativ und untergräbt jede pädagogische Absicht, die Ihr Kommentar gehabt haben könnte.

In ähnlicher Weise ist "Ich hätte das anders gemacht ..." auch konfrontativ, da der Entwickler sofort davon ausgeht, dass " Nun, ich habe es so gemacht ... Sie haben ein Problem damit? " es muss sein und lenkt die Diskussion von der Verbesserung des Codes ab.

Beispiel:

Anstatt zu fragen , „Warum haben Sie nicht die konstante Variable für diesen Wert zu benutzen?“, Einfach sagen „Dieser hartcodierte Wert sollte mit der Konstante ersetzt wird XYZin der Datei Constants.h“ die Frage stellen läßt vermuten , dass der Entwickler aktiv wählte nicht die verwenden bereits definierte Konstante, aber es ist durchaus möglich, dass sie nicht einmal wussten, dass es existiert. Mit einer ausreichend großen Codebasis wird es eine Menge Dinge geben, die jeder Entwickler nicht weiß. Dies ist einfach eine gute Gelegenheit für den Entwickler, sich mit den Konstanten des Projekts vertraut zu machen.

Es gibt eine feine Linie, um mit Codeüberprüfungen umzugehen: Sie müssen nicht alles, was Sie sagen, mit Zucker überziehen, Sie müssen keine schlechten Nachrichten mit guten Nachrichten kombinieren und Sie müssen den Schlag nicht mildern. Es könnte die Kultur sein, in der ich mich befinde, aber meine Kollegen und ich gratulieren uns nie bei Code-Überprüfungen - die Teile des Codes, die wir entwickeln, die fehlerfrei sind, sind genug von einem Kompliment. Was Sie tun müssen, ist, den Defekt zu identifizieren, den Sie sehen, und vielleicht einen Grund anzugeben (das Warum ist weniger nützlich, wenn es offensichtlich / einfach ist).

Gut:

• "Der Name dieser Variablen muss geändert werden, um unserem Kodierungsstandard zu entsprechen."

• "Das 'b' in diesem Funktionsnamen muss groß geschrieben werden, um PascalCase zu sein."

• "Der Code dieser Funktion wird nicht richtig eingerückt."

• "Dieser Code ist ein Duplikat des Codes in ABC::XYZ()und sollte stattdessen diese Funktion verwenden."

• "Sie sollten try-with-resources verwenden, damit die Dinge in dieser Funktion garantiert ordnungsgemäß geschlossen werden, auch wenn Fehler auftreten." [Ich habe hier nur einen Link hinzugefügt, damit Nicht-Java-Benutzer wissen, was Try-with-Resources bedeutet.]

• "Diese Funktion muss überarbeitet werden, um unsere n-Pfad-Komplexitätsstandards zu erfüllen."

Schlecht:

• "Ich denke, Sie könnten diesen Code verbessern, indem Sie den Variablennamen so ändern, dass er unserem Standard entspricht."

• " Vielleicht ist es besser, try-with-resource zu verwenden, um die Dinge in dieser Funktion ordnungsgemäß zu schließen."

• "Es könnte wünschenswert sein, alle Bedingungen in dieser Funktion noch einmal genauer zu betrachten. Ihre N-Pfad-Komplexität liegt über der maximal zulässigen Komplexität unseres Standards."

• "Warum sind die Einrückungen hier 2 Leerzeichen anstelle der 4 unseres Standards?"

• "Warum haben Sie eine Funktion geschrieben, die unseren n-Pfad-Komplexitätsstandard verletzt?"

In den schlechten Aussagen sind die kursiv geschriebenen Teile Dinge, die Menschen üblicherweise verwenden, um den Schlag zu mildern, aber alles, was sie bei einer Codeüberprüfung wirklich tun, ist, dass Sie sich unsicher klingen. Wenn Sie als Rezensent nicht sicher sind, wie Sie den Code verbessern können, warum sollte dann jemand auf Sie hören?

Die "guten" Aussagen sind unverblümt, aber sie beschuldigen den Entwickler nicht, sie greifen den Entwickler nicht an, sie sind nicht konfrontativ und sie hinterfragen nicht, warum der Fehler vorliegt. Es existiert; Hier ist die Lösung. Sie sind sicherlich nicht so konfrontativ wie diese letzte "Warum" -Frage.

Das Problem, das Sie sehen, ist: Entwickler sind unglücklich, dass ihr Code kritisiert wird. Aber das ist nicht das Problem. Das Problem ist, dass ihr Code nicht gut ist (vorausgesetzt natürlich, dass Ihre Codeüberprüfungen gut sind).

Wenn die Entwickler ihren Code nicht mögen, der Kritik erregt, ist die Lösung einfach: Schreiben Sie besseren Code. Sie sagen, Sie hatten ernsthafte Probleme mit der Codequalität. Das bedeutet, dass ein besserer Code benötigt wird.

"Warum sollte die Methode einen vernünftigen Namen haben, ich weiß was sie tut?" finde ich besonders störend. Er weiß, was es tut, oder zumindest sagt er es, aber ich nicht. Jede Methode sollte nicht nur einen vernünftigen Namen haben, sondern auch einen Namen, der dem Leser des Codes sofort klar macht, was er tut. Vielleicht möchten Sie die Apple-Website besuchen und nach einem WWDC-Video über die Änderungen von Swift 2 zu Swift 3 suchen - eine Vielzahl von Änderungen, die vorgenommen wurden, um alles besser lesbar zu machen. Vielleicht könnte diese Art von Video Ihre Entwickler davon überzeugen, dass Entwickler, die viel schlauer sind als sie, intuitive Methodennamen für sehr, sehr wichtig halten.

Ein anderes störendes Element war Ihre Kollegin, die sagte: "Sie hat mir selbst gesagt, dass sie, nachdem ein Fehlerbericht von einem Kunden eingereicht wurde, von dem Fehler wusste, aber befürchtete, dass der Entwickler sauer auf sie sein würde, weil sie darauf hingewiesen hat." Es besteht die Möglichkeit, dass es Missverständnisse gibt, aber wenn ein Entwickler wütend wird, wenn Sie ihn auf einen Fehler hinweisen , ist das schlimm.

Ich würde der von Ihnen beschriebenen Methode der Codeüberprüfung mit Respekt widersprechen. Zwei Mitarbeiter, die sich in einen „geschlossenen Raum“ begeben und Kritik üben, institutionalisieren die gegnerische Vorstellung, dass gute Code-Überprüfungen vermieden werden sollten .

Um erfolgreich zu sein, ist es unerlässlich, die Codeüberprüfung so weit wie möglich positiv zu gestalten. Schritt eins ist, den kontroversen Begriff der Überprüfung zu entfernen. Machen Sie es zu einem wöchentlichen oder zweiwöchentlichen Gruppenevent . Stellen Sie sicher, dass jeder weiß, dass er zur Teilnahme eingeladen ist. Bestellen Sie in Pizza oder Sandwiches oder was auch immer. Nennen Sie es nicht einmal eine "Codeüberprüfung", wenn dies negative Konnotationen hervorruft. Finde etwas zum Feiern, Ermutigen, Teilen - auch wenn es nichts anderes ist als ein Fortschritt durch den aktuellen Sprint oder die aktuelle Iteration. Abwechselnd die Führung über die Überprüfung zuweisen.

Machen Sie den Prozess zu einem Bestreben, dem Produkt und den Menschen zu dienen. Wenn sie konstruktiv und positiv durchgeführt werden, wobei gute Techniken oder Muster geteilt und gefördert werden, so wie arme entmutigt werden - profitieren alle. Jeder wird in der Öffentlichkeit trainiert. Wenn Sie einen Problemprogrammierer haben, der es nicht "versteht", sollten Sie ihn privat und getrennt ansprechen - niemals vor der breiteren Gruppe. Das ist getrennt von der Codeüberprüfung.

Wenn Sie Probleme haben, etwas "Gutes" zu finden, dann stellt sich für mich die Frage: Wenn Fortschritte im Projekt erzielt werden und die Leute arbeiten, ist dieser Fortschritt an und für sich ein Grund zum Feiern. Wenn Sie wirklich nichts Gutes finden, bedeutet dies, dass alles, was seit der letzten Überprüfung getan wurde, entweder schlecht oder nicht besser als neutral ist . Ist das wirklich der Fall?

Bei den Details sind gemeinsame Standards unerlässlich. Geben Sie jedem einen Anteil an dem, was getan wird. Außerdem können neuere, bessere Techniken in Ihre Codebasis integriert werden. Das Versäumnis, dies zu tun, garantiert, dass alte Gewohnheiten nie unter dem Deckmantel "wir haben es immer so gemacht" entfernt werden.

Der springende Punkt dabei ist, dass Code-Reviews ein bisschen schlecht aufgenommen werden, weil sie schlecht implementiert sind, als Hämmer verwendet werden, um weniger erfahrene oder weniger erfahrene Programmierer herabzusetzen, und das nützt niemandem. Manager, die sie auf diese Weise einsetzen, sind wahrscheinlich auch keine sehr effektiven Manager. Gute Codeüberprüfungen, bei denen jeder Teilnehmer ist, dienen allen - dem Produkt, den Mitarbeitern und dem Unternehmen.

Wir entschuldigen uns für die Länge des Beitrags, aber da wir in einer Gruppe waren, in der die Codeüberprüfung weitgehend ignoriert wurde, führte dies zu einem Vermächtnis von nicht wartbarem Code und nur einer begrenzten Anzahl von Entwicklern, die in der Lage waren, Änderungen an einer jahrelangen Codebasis vorzunehmen. Es war ein Weg, den ich nicht noch einmal beschließen würde.

Das Paradoxon für guten Code ist, dass er überhaupt nicht auffällt und anscheinend sehr einfach und einfach zu schreiben war. Ich mag das Poolspieler-Beispiel aus dieser Antwort wirklich . Das macht es in Code-Reviews wirklich einfach, zu beschönigen, es sei denn, es handelt sich um einen Refactor für fehlerhaften Code.

Ich lege Wert darauf, danach zu suchen. Wenn ich Code überprüfe und eine Datei durchgesehen habe, die so einfach zu lesen war, dass sie täuschend einfach zu schreiben scheint, gebe ich einfach ein kurzes "Ich mag, wie die Methoden hier kurz und sauber sind" oder was auch immer angemessen ist .

Sie sollten auch mit gutem Beispiel vorangehen. Sie sollten darauf bestehen, dass auch Ihr Code überprüft wird, und Sie sollten modellieren, wie sich Ihr Team bei der Korrektur verhalten soll. Am wichtigsten ist jedoch, dass Sie sich aufrichtig für das Feedback bedanken. Dies macht mehr Unterschied als jedes einseitige Feedback, das Sie geben können.

Es hört sich so an, als ob das eigentliche Problem darin besteht, dass nur zwei Personen alle Code-Überprüfungen durchführen, von denen nur Sie sie ernst nehmen Druck von den anderen Teammitgliedern.

Besser wäre es, die Verantwortung für die Codeüberprüfung auf das Team zu verteilen und alle an der Überprüfung teilnehmen zu lassen, indem beispielsweise die Entwickler entscheiden, wer ihren Code überprüfen soll. Dies sollte Ihr Codeüberprüfungstool unterstützen.

Ich habe das hautnah miterlebt und möchte Sie vor den emotionalen Intelligenzfragen warnen - sie können ganze Teams töten. Wenn Sie nicht jedes Jahr neue Entwickler einstellen, schulen und normalisieren möchten, ist es wichtig, ein positives Verhältnis zu Ihren Kollegen aufzubauen. Ist es nicht der springende Punkt, diese Überprüfungen durchzuführen, um die Codequalität zu verbessern und eine Kultur zu fördern, in der die Codequalität in erster Linie höher ist? Ich würde argumentieren, dass es in der Tat Teil Ihrer Aufgabe ist, eine positive Verstärkung bereitzustellen, um dieses Code-Überprüfungssystem in die Teamkultur zu integrieren.

Wie auch immer, hört sich so an, als müssten Sie Ihr Code Review-System überprüfen. Momentan ist nach den Klängen alles in Ihrem Überprüfungsprozess eher subjektiv als objektiv oder kann als subjektiv interpretiert werden. Es ist leicht, verletzte Gefühle zu bekommen, wenn jemand das Gefühl hat, Ihren Code einfach auseinanderzunehmen, weil er ihn nicht mag, anstatt einen Grund dafür zu haben, dass er zitieren kann, wenn etwas nicht den Richtlinien entspricht. Auf diese Weise ist es einfach, positive Verbesserungen der Codequalität (in Bezug auf Ihr Überprüfungssystem) zu verfolgen und zu feiern, wie es für Ihre Bürokultur angemessen ist.

Die Technical Leads müssen sich außerhalb einer Überprüfungssitzung zusammenfinden und eine Codierungsrichtlinie / Code-Überprüfungs-Checkliste herausgeben. Diese muss während der Überprüfung eingehalten und angegeben werden. Dies sollte ein lebendiges Dokument sein, das im Verlauf des Prozesses aktualisiert und verfeinert werden kann. Diese Tech-Lead-Besprechungen sollten auch stattfinden, wenn die Diskussion "Wie wir immer Dinge getan haben" im Vergleich zu "Neue und verbesserte Methoden" stattfinden soll, ohne dass der Entwickler das Gefühl hat, dass die Meinungsverschiedenheit ein Ergebnis seines Codes ist. Sobald die anfängliche Richtlinie mehr oder weniger geglättet wurde, können die Entwickler auch positiv gestärkt werden, indem sie um ihr Feedback gebeten und dann tatsächlich Maßnahmen ergriffen werden Wenn Sie den Prozess als Team weiterentwickeln, ist dies ein Wunder, wenn Sie sie auf den neuesten Stand bringen und damit beginnen, den Code für Onboards zu überprüfen.

Lokal...

Programmierer sind Problemlöser. Wir sind darauf vorbereitet, mit dem Debuggen zu beginnen, wenn ein Problem oder ein Fehler auftritt.

Code ist ein Medium im Umrissformat. Das Wechseln zu einer Erzählung im Absatzformat für Feedback führt zu einer Unterbrechung, die übersetzt werden muss. Unweigerlich geht etwas verloren oder wird missverstanden. Es ist unvermeidlich, dass der Rezensent nicht in der Sprache des Programmierers spricht.

Computergenerierte Fehlermeldungen werden selten in Frage gestellt und niemals als persönliche Beleidigung gewertet.

Code-Überprüfungselemente sind vom Menschen erzeugte Fehlermeldungen. Sie sollen die Randfälle auffangen, die Programmierer und automatisierte Tools übersehen, sowie die unvermeidlichen Nebenwirkungen auf andere Programme und Daten.

Schlussfolgerungen ...

Je mehr Code-Überprüfungselemente in automatisierte Tools integriert werden können, desto besser werden sie empfangen.

Die Einschränkung ist, dass solche Tools, um unbestritten zu bleiben, regelmäßig täglich oder wöchentlich aktualisiert werden müssen, um jeder Änderung an Verfahren, Standards und Praktiken zu entsprechen. Wenn ein Manager oder Entwicklerteam eine Änderung vornimmt, müssen die Tools geändert werden, um dies durchzusetzen. Ein Großteil der Arbeit des Code-Reviewers besteht dann darin, die Regeln in den Tools beizubehalten.

Beispiel Code Review Feedback ...

Eine Umschreibung des angegebenen Beispiels mit den folgenden Techniken:

• Gegenstand:

  • Library \ ACME \ ExtractOrderMail-Klasse.

• Grundsatzfrage ...

  • TempFilesToDelete ist statisch
    • Nachfolgende Aufrufe von GetMails lösen eine Ausnahme aus, da Dateien hinzugefügt, aber nach dem Löschen nie entfernt werden. Obwohl dies jetzt nur ein Aufruf ist, könnte die Leistung in Zukunft mit einer gewissen Parallelität verbessert werden.
    • Mit TempFilesToDelete als Instanzvariable können mehrere Objekte gleichzeitig verwendet werden.
• Nebenausgaben ...
  • GetErrorMailBody hat einen Ausnahmeparameter
    • Ist es notwendig, da es selbst keine Ausnahme auslöst und diese nur an ToString übergibt?
  • SaveAndSend Name
    • Möglicherweise wird dies in Zukunft per E-Mail gemeldet oder nicht. Dieser Code enthält die allgemeine Logik zum Speichern einer dauerhaften Kopie und zum Melden von Fehlern. Ein allgemeinerer Name würde solche vorweggenommenen Änderungen zulassen, ohne die abhängigen Methoden zu ändern. Eine Möglichkeit ist StoreAndReport.
  • Auskommentieren von altem Code
    • Das Kommentieren und Markieren einer Zeile mit OBSOLETE kann beim Debuggen sehr hilfreich sein, aber eine "Kommentarwand" kann auch Fehler im angrenzenden Code verdecken. Wir haben es immer noch in Subversion. Vielleicht nur ein Kommentar, der genau darauf verweist, wo in Subversion?

Wenn Sie nichts Nettes über den vorhandenen Code zu sagen haben (was verständlich klingt), versuchen Sie, den Entwickler in die Verbesserung einzubeziehen.

In einem Patch für eine funktionale Änderung oder einen Bugfix ist es nicht hilfreich, andere Änderungen (Umbenennen, Umgestalten usw.) in demselben Patch zusammenzufassen. Es sollte die Änderung vornehmen, die den Fehler behebt oder die Funktion hinzufügt, sonst nichts.

Wo Umbenennung, Refactoring und andere Änderungen werden angezeigt, tun sie in einem separaten Patch, vor oder nach.

• Das ist ziemlich hässlich, aber ich denke, es stimmt mit all unseren anderen bösen Codes überein. Lassen Sie uns das später bereinigen (in einem Patch, der im Idealfall keine funktionalen Änderungen enthält).

  Es hilft auch , wenn Sie in Refactoring beitreten, und lassen Sie sie überprüfen Ihren Code. Es gibt Ihnen die Möglichkeit zu sagen, warum Sie etwas für gut und nicht für schlecht halten, und ein Beispiel dafür zu zeigen, wie Sie konstruktive Kritik gut aufnehmen.

Wenn Sie einer neuen Funktion Komponententests hinzufügen müssen, werden diese im Hauptpatch aufgeführt. Wenn Sie jedoch einen Fehler beheben, sollten die Tests in einem früheren Patch durchgeführt und nicht bestanden werden, damit Sie nachweisen können, dass der Fehler tatsächlich behoben wurde.

Im Idealfall sollte der Plan (wie Sie einen Fix testen oder was Sie korrigieren und wann) vor Abschluss der Arbeit besprochen werden, damit sich die Mitarbeiter nicht viel Mühe geben, bevor Sie feststellen, dass Sie ein Problem damit haben.

Wenn Sie feststellen, dass der Code nicht mit Eingaben zurechtkommt, die Sie für erforderlich halten, kann dies auch zu Abweichungen bei den vom Entwickler als sinnvoll erachteten Eingaben führen. Stimmen Sie dies, wenn möglich, auch im Voraus zu. Sprechen Sie wenigstens darüber, mit was es fertig werden soll und wie schlimm es sein kann, wenn es versagt.

Ich halte es für einen Fehler anzunehmen, dass es eine technische oder einfache Lösung gibt: "Meine Mitarbeiter sind verärgert, wenn ich ihre Arbeit nach meinen Maßstäben beurteile und die Befugnis habe, sie durchzusetzen".

Denken Sie daran, dass Code-Reviews nicht nur eine Diskussion darüber sind, was gut oder schlecht ist. Sie sind implizit ein "Wer ist der Maßstab, den wir verwenden", ein "Wer trifft die Entscheidung" und daher - höchst heimtückisch - ein Rang.

Wenn Sie diese Punkte nicht ansprechen, wird es schwierig sein, Codeüberprüfungen so durchzuführen, dass die Probleme, auf die Sie stoßen, nicht auftreten. Hier gibt es einige gute Ratschläge, aber Sie haben sich eine schwierige Aufgabe gestellt.

Wenn Sie Recht haben, ohne Spielraum, ist es ein Angriff, darauf hinzuweisen: Jemand hat es vermasselt. Wenn nicht: Ihr anderer MBA, der es nicht bekommt. Wie auch immer, du wirst der Böse sein.

Wenn jedoch klar und einverstanden ist, worauf die Überprüfung hinausläuft und warum , haben Sie eine gute Chance, nicht der Bösewicht zu sein. Sie sind nicht der Torhüter, nur ein Korrektor. Es ist schwierig, diese Vereinbarung zu erreichen [1]. Ich würde Ihnen gerne einen Rat geben, aber ich habe selbst noch keine Ahnung ...

[1] Es ist nicht gelöst, nur weil die Leute "ok" gesagt haben oder aufgehört haben, darüber zu streiten. Keiner möchte der Typ sein, der sagt, dass X für unsere {Intelligenz, Erfahrung, Personalstärke, Fristen usw.} einfach nicht praktikabel ist, aber das bedeutet nicht, dass es tatsächlich um eine bestimmte Instanz von X geht ...

Code Reviews sollten gegenseitig sein. Jeder kritisierte jeden. Das Motto lautet "Sei nicht böse, sei ausgeglichen." Jeder macht Fehler und wenn die Leute einmal einen Hot Shot gemacht haben, wie sie ihren Code reparieren können, werden sie verstehen, dass dies nur das normale Verfahren ist und kein Angriff auf sie.

Den Code anderer Leute zu sehen, ist lehrreich. Wenn Sie den Code so gut verstehen, dass Sie auf seine Schwachstelle hinweisen können, und erklären müssen, dass Schwachstelle Ihnen viel beibringen kann !

Es gibt wenig Raum für Lob in einer Codeüberprüfung, da der springende Punkt darin besteht, Fehler zu finden. Sie können jedoch Lob für Fixes sammeln . Sowohl Aktualität als auch Ordentlichkeit können gelobt werden.