ALLE Entwickler dazu bringen, Codeüberprüfungen durchzuführen

13

Ich bin ein Softwareentwickler in einem 7-8 Entwicklerteam. Wir führen seit geraumer Zeit Code-Reviews durch und die Code-Qualität hat sich im Laufe der Zeit verbessert.

Allerdings habe ich kürzlich festgestellt, dass einige Entwickler mehr Code-Reviews als die anderen erhalten. Ich fürchte, das liegt an ihrer flexiblen Einstellung.

Meiner Ansicht nach sollte Code-Review nicht so durchgeführt werden: Das gesamte Team sollte dafür verantwortlich sein, und Code-Reviewer sollten nicht aufgrund ihrer Bereitschaft ausgewählt werden, Änderungen einfach zu akzeptieren.

Wie gehen Sie in Ihrem Team mit diesem Problem um?

Haben Sie Regeln für die Auswahl von Code-Reviewern festgelegt?

Denken Sie, dass Code-Reviewer für die Zeit, die sie mit (guten) Code-Reviews verbringen, belohnt werden sollten? Und wie sollen sie belohnt werden?

Vielen Dank für Ihre Antworten / Ideen.

guillaumegallois
quelle
7
Haben Sie darüber nachgedacht, ein Round-Robin-System zu erstellen, bei dem sowohl der Codierer darüber im Dunkeln gelassen wird, wer überprüft, als auch der Überprüfer darüber, wer der Codierer ist, im Dunkeln gelassen wird?
Neil
1
Ich habe nicht, aber ich mag diese Idee! Vielen Dank!
Guillaumegallois
1
Wer ist verantwortlich und warum erledigen sie ihre Arbeit nicht, was bedeuten sollte, dass sichergestellt wird, dass alle anderen ihre Arbeit tun?
JeffO
In meinem Team werden Prüfer automatisch zugewiesen, wenn eine Pull-Anfrage geöffnet wird. Die Reviewer werden aus dem Round-Robin-Team ausgewählt. Wir haben einen Webhook für jedes unserer Repos, der automatisch Prüfer zuweist, wenn die PR geöffnet wird. Es führt im Grunde genommen eine Liste aller Entwickler und der zuletzt zugewiesenen und arbeitet sich einfach durch die Liste.
Dan Jones

Antworten:

12

Wir wählen keine Gutachter aus.

In unserem Team:

  • Alle Codeänderungen müssen vor Abschluss der Pull-Anforderung überprüft werden
  • Mindestens ein Entwickler muss den Code überprüfen (zwei oder mehr Überprüfer sind besser und es ist auch äußerst vorteilhaft, wenn Tester, Analysten und andere Teammitglieder den Code überprüfen).

Code Reviews werden nicht zugewiesen, sondern abgeholt, wenn es für sie funktioniert. Das Verständnis ist, dass wir keine Geschichten durch die Pipeline schieben können. Gelegentlich wird jemand erwähnen, dass er auf eine CR im Standup wartet, aber das war es auch schon.

Ich mag dieses Modell, es gibt den Leuten die Möglichkeit aufzuheben, was sie können, und vermeidet es, "den Leuten Jobs zu geben".

Liath
quelle
6

Führen Sie eine Regel ein, nach der ein Fehler behoben werden kann, und zwar nicht nur für diejenigen, die die Änderung übernommen haben, sondern nur für diejenigen, die sie überprüft haben. Das sollte die richtige Perspektive für die Codeüberprüfung schaffen.

Wie für,

Denken Sie, dass Code-Reviewer für die Zeit, die sie mit (guten) Code-Reviews verbringen, belohnt werden sollten?

Nun, ich bin mir nicht sicher, wie Entwickler im Allgemeinen für ihre Arbeit belohnt werden, außer wenn sie nur ein Gehalt bekommen und irgendwie stolz auf das sind, was sie getan haben. Da die Überprüfung von Code Teil ihrer Aufgabe ist, sollte der Prüfer Zeit für die Überprüfung erhalten und das Guthaben auf irgendeine Weise teilen.

max630
quelle
2
Es ist eine interessante Idee, aber oft, wenn ein Fehler auftritt, finden 90% der Arbeit genau heraus, was den Fehler verursacht, und 10% der Arbeit beheben ihn. Ein Post-Mortem-Verfahren durchzuführen, um genau herauszufinden, welche Änderung den Fehler verursacht hat, kann nicht einmal passieren, es sei denn, es hilft herauszufinden, was vor sich geht oder wie eine sichere Lösung durchgeführt werden kann.
DaveG
Sie haben die Anerkennung, die Code-Reviewer erhalten sollten, gut begründet. Dies ist definitiv ein Problem, das angegangen werden sollte. Danke für deine Antwort!
Guillaumegallois
Ich denke, es könnte Leute dazu bringen, zu versuchen, überhaupt keine Codeüberprüfungen durchzuführen, oder es könnte sein, dass Sie keine Arbeit erledigen lassen, weil die Leute anfangen, mit der Auswahl zu beginnen.
Mateusz
5
Diese Antwort geht davon aus, dass das Ziel der Codeüberprüfung darin besteht, Fehler zu finden. Das ist nicht der Fall, das primäre Ziel ist es, den Code in einem wartbaren und entwicklungsfähigen Zustand zu halten (und wenn Sie Glück haben, werden auch einige Fehler gefunden).
Doc Brown
@DocBrown, um Fehler zu vermeiden und auch um sicherzustellen, dass der Fehler in Zukunft schneller behoben wird (indem der andere Entwickler mit dem Code vertraut gemacht und sichergestellt wird, dass der Code gut geschrieben ist)
max630
4

Das Hauptproblem, das Sie haben, ist nicht technisch, aber einige Tools können den Aufwand für Codeüberprüfungen verringern. Es gibt ein paar Herausforderungen zu meistern:

  • Verstehen, was die Änderung ist. Git Pull Requests für Feature-Zweige unterstützen diesen Prozess.
  • Durchführen einer Codeüberprüfung zu einem Ereignis, bei dem sich Personen im selben Raum befinden müssen. Dies erhöht die Belastung durch Zeitplanung, Besprechungsressourcen usw. GitHub, GitLab und BitBucket ermöglichen asynchrone Überprüfungen, damit sie ausgeführt werden können, wenn der Peer bereit ist.
  • Die Fähigkeit, beim Betrachten von Code ein aussagekräftiges Feedback zu geben. Um ehrlich zu sein, ist die Funktion zum zeilenweisen Kommentieren in GitHub-, GitLab- und Bitbucket-Pull-Anfragen wirklich nützlicher als ein persönliches Treffen. Es fühlt sich weniger politisch an.

Das soll nicht heißen, dass Sie nicht SubVersion oder andere Tools (wie Fisheye) verwenden können, aber die in die Git-Pipeline integrierte Integration mit Feature-Verzweigungen macht die Arbeit wirklich weniger mühsam.

Außerhalb des Werkzeugbaus müssen Sie sich mit weiteren sozialen Herausforderungen befassen:

  • Lassen Sie Ihre Entwickler in den Tag starten, indem Sie alle offenen Pull-Anforderungen überprüfen, um sich abzumelden.
  • Lassen Sie Ihre Entwickler alle offenen Pull-Anforderungen überprüfen, bevor sie eine neue Aufgabe starten
  • Für Ihre Pull-Anfragen ist die Genehmigung von mehr als einer Person erforderlich.

Es könnte sich auch lohnen, zu prüfen, welche Arten von Aufgaben von den engagierteren Personen überprüft werden. Möglicherweise greifen sie zu all den einfachen Überprüfungen, was die Situation für alle anderen schmerzhafter macht.

Berin Loritsch
quelle
Der letzte Punkt ist gut. Ich habe einmal mit einem Entwickler in einem kleinen Team zusammengearbeitet, das nur Änderungen an der von ihm geschriebenen Software überprüfte, die an anderer Stelle im Team zu erheblichen Engpässen führten.
Robbie Dee
In diesem Fall scheint es, als ob Sie jemanden haben, der versucht, sein "Territorium" zu schützen. Sie möchten so weit wie möglich das Gemeinschaftsgefühl für den Code fördern. Mit anderen Worten, es gibt keine geschützten Schutzgebiete innerhalb des Codes, die kein anderer Entwickler außer dem "Gesegneten" berühren kann. Ich verstehe, wenn es eine spezielle Lücke gibt und Sie die Mathematik nicht überprüfen können, aber Sie können zumindest überprüfen, wie gut der Code mit seiner Absicht übereinstimmt.
Berin Loritsch
2

Eine gute Idee ist es, dies auf Round-Robin-Basis zu tun - Sie wählen jemanden aus, der die geringste Anzahl von Überprüfungen für Ihren Code durchgeführt hat. Im Laufe der Zeit sollte jeder im Team ungefähr die gleiche Anzahl von Überprüfungen durchgeführt haben. Es verbreitet auch das Wissen.

Offensichtlich wird es gelegentlich Ausnahmen wie Feiertage geben, an denen es zu Höhen und Tiefen kommt.

Es sollte keine Unterscheidung zwischen Junioren und Senioren / Leads geben. Codeüberprüfungen sollten für jeden Code durchgeführt werden - egal wie alt er ist. Es reduziert die Reibung und hilft, unterschiedliche Ansätze zu teilen.

Wenn Sie sich nach alledem immer noch Sorgen über die Qualität der Überprüfungen machen, sollten Sie eine Reihe von Mindeststandards einführen, damit eine Codeüberprüfung erfolgreich ist. Was Sie einbeziehen, liegt ganz bei Ihnen, aber einige Dinge, die Sie in Betracht ziehen könnten, sind Codeabdeckung, Komponententests, Entfernen von auskommentiertem Code, Metriken, ausreichende Kommentare, Verarbeitungsqualität, SOLID-Prinzipien, DRY, KISS usw.

Für die Überprüfung von Anreizen ist der Qualitätscode die Belohnung. Ich bin mir sicher, dass wir an suboptimalen Codebasen gearbeitet haben, bei denen der Schmerz erheblich hätte verringert werden können, wenn ein anderer Entwickler den Code von Anfang an neu erstellt und konstruktive Änderungen vorgeschlagen hätte.

Robbie Dee
quelle
0

Es hört sich so an, als ob dem Team ein formaler Prozess für die Codeüberprüfung fehlt.

Ich spreche nicht von der Erstellung eines 350-seitigen Word-Dokuments, sondern von ein paar einfachen Aufzählungspunkten.

Die wichtigen Punkte:

  1. Definieren Sie eine Kernmenge von Prüfern. Keine allgemeinen Aussagen. Nennen Sie Leute.

    Dies sollten Ihre leitenden Entwickler sein.

  2. Fordern Sie mehr als einen Hauptprüfer auf, um die Überprüfung abzuschließen.

  3. Identifizieren Sie mindestens einen anderen Nicht-Kernprüfer pro Sprint oder Release, der ein temporärer Kernprüfer ist. Erfordern die Abmeldung bei allen Codeüberprüfungen während dieser Zeit.

Element 3 ermöglicht den anderen Entwicklern, sich in der Hauptprüfergruppe zu drehen. In einigen Wochen verbringen sie mehr Zeit mit Bewertungen als in anderen. Es ist ein Spagat.

Wie für die Belohnung von Menschen? Viele Male zugeben, dass der Aufwand, den eine Person während der Codeüberprüfung vor dem gesamten Team unternimmt, funktionieren kann, aber Sie sollten sich nicht darüber streiten.

Definieren Sie im Zweifelsfall den Prozess und teilen Sie dem Team mit, dass es sich daran halten muss.

Und es gibt noch eine letzte Sache, die Sie versuchen können - so kontrovers es auch sein mag: Lassen Sie das @ # $% den Fan treffen, wenn ich ein Idiom verwenden darf.

Lassen Sie das Team scheitern, da der Codeüberprüfungsprozess nicht befolgt wird. Das Management wird sich engagieren, und dann werden sich die Leute ändern. Dies ist wirklich nur eine gute Idee in den extremsten Fällen, in denen Sie bereits einen Prozess definiert haben und das Team sich geweigert hat, sich daran zu halten. Wenn Sie nicht die Befugnis haben, Personen zu entlassen oder zu disziplinieren (wie die meisten führenden Entwickler dies nicht tun ), müssen Sie jemanden einbeziehen, der diese Aufgaben ausführen kann.

Und es gibt nichts Schöneres, als Dinge nicht ändern zu können. Trotz allem , was könnte die Leute sagen, Sie können die Titanic steuern - aber nicht , bevor er trifft das Eis burg.

Manchmal muss man einfach die Titanic auf die Eisburg schlagen lassen.

Greg Burghardt
quelle