Wann soll der Code überprüft werden? Vor oder nach dem Check-in zu MAIN? [geschlossen]

8

Beim Durchsuchen von stackoverflow habe ich eine Reihe von Posts bemerkt, in denen behauptet wird, dass einige Entwickler eine Code-Überprüfung vor dem Check-in für MAIN befürworten. Kann dies jedoch wirklich durchgesetzt werden? Und wenn dies der Fall ist, wird die Wahrscheinlichkeit, dass Code aufgrund des erhöhten Aufwands umstrukturiert wird, sicherlich reduziert.

Persönlich bevorzuge ich den Ansatz, kontinuierliche Integration einzusetzen, um jeden zu fangen, der den Build bricht, und am Ende einer bedeutenden Arbeit Code-Reviews auf einer grobkörnigeren Basis durchzuführen. Ich vergebe keine Code-Reviews für kleinere Refactorings.

Ich bin daran interessiert, Ihre Meinung zu hören.

    
toolkit 18.09.2008, 21:28
quelle

16 Antworten

4

Alles hängt davon ab, wie Ihre Organisation eingerichtet ist.

Für kleine Teams ist die Strategie, die ich befürworte, dass alle Checkins direkt in Ihrem Kofferraum sein sollten, Diffs sollten an das Team geschickt werden, und dass alle Checkins vom Team aus diesen Diffs überprüft werden sollten. Dies hat wenig Overhead und ermutigt viele kleine Checkins (die einfacher per E-Mail zu überprüfen sind). Abhängig von Ihrer Gruppe wird es wahrscheinlich rotieren, wer die Hauptverantwortung für die Überprüfung des Codes zu einem bestimmten Zeitpunkt trägt. Sie verzweigen dann Testfreigaben aus dem Stamm, senden sie über QA und geben sie dann zur Produktion frei, nachdem die QA sie genehmigt hat.

Größere Teams entwickeln wahrscheinlich in mehreren Zweigen. Je nachdem, wie Sie organisiert sind, haben Sie vor dem Einchecken Möglichkeiten zur Codeüberprüfung (funktioniert mit der Paarprogrammierung), vor dem Zusammenführen (das ist effektiv, wie viele Open Source-Projekte funktionieren) oder zu regelmäßig geplanten Punkten in Ihrem Entwicklungszyklus (das wäre formellere Kritiken).

Der Hauptunterschied besteht darin, dass es bei kleinen Teams darum geht, den Prozess zu reduzieren, während bei großen Teams nur der Kommunikationsaufwand kontrolliert wird. Aber lassen Sie die kleine Teamdynamik nicht als Unprofessionalität abtun. Kleine Teams genießen / große Produktivitätsvorteile gegenüber großen. Lawrence Putnam hat einige faszinierende Nachforschungen darüber angestellt, die Sie in Software Estimation: Demystifying the Black Art finden können Steve McConnell (siehe Seite 229). Er fand heraus, dass für mittelgroße Projekte (~ 50.000 Codezeilen) das durchschnittliche 5-7-Personen-Team im weniger Kalenderjahr abschließen wird als ein durchschnittliches Team der Größe 15-20. Ich würde wetten, dass das kleine Team einen kohärenteren Code hat und wahrscheinlich mehr erreicht hat.

Denken Sie daran, dass dies durchschnittliche Teams sind. Beachten Sie, dass das Auffinden von 5 außergewöhnlichen Personen einfacher ist als das Auffinden von 20 von ihnen. Es ist daher einfacher, außergewöhnliche kleine Teams als außergewöhnliche große Teams zu erstellen. Angesichts der dokumentierten individuellen Produktivitätsunterschiede würde ich ernsthaft darauf wetten, dass ein außergewöhnliches Team von 5 Personen leicht einen höheren Durchsatz haben wird als ein durchschnittliches Team von 100 Personen. Und ihre Bearbeitungszeit bei kleinen Projekten wird viel schneller sein. Selbst wenn Sie sie viermal so viel pro Person bezahlen, ist das immer noch 1/5 der Kosten!

    
user11318 18.09.2008, 22:41
quelle
3

Meiner Meinung nach sollten Sie jedes Mal, wenn Sie einchecken, einen Code-Review haben. Die Kosten für das Reparieren steigen ein bisschen, wenn es im Baum ist.

    
Bill Wert - MSFT 18.09.2008 21:30
quelle
1
  1. Erledige die gesamte Entwicklung in einer Filiale und verpflichte dich so, wie du willst.
  2. Code Überprüfen Sie die Änderungen, sobald die gesamte Entwicklung abgeschlossen ist.
  3. Übergeben Sie dann die Verzweigung an Testen.
  4. Wenn der Branchentest abgeschlossen ist, führen Sie den Code in den Release Candidate-Zweig ein.
  5. Der Zweig Release Candidate wird nach jeder einzelnen Zusammenführung einer Regressionsprüfung unterzogen.
  6. Abschließende QA- und UA-Tests, die mit RC durchgeführt wurden, nachdem alle Dev-Zweige zusammengeführt wurden.
  7. Sobald QA und UAT übergeben wurden, verschmelzen Sie den Release-Zweig in den MAIN / TRUNK-Zweig.

Wir überprüfen Code Review derzeit manuell, aber ich dränge darauf, Atlassian Crucible + FishEye einzurichten, was den Prozess viel einfacher macht.


Die Paarprogrammierung kann nützlich sein, wenn zwei Personen mit unterschiedlichen Perspektiven an einem bestimmten Problem arbeiten. Oder einen Entwickler über Code zu unterrichten, mit dem er nicht vertraut ist, aber der andere weiß es gut. Aber für die allgemeine Entwicklung kann es sehr zeitaufwendig sein und verschwendet Ressourcen, die an zwei verschiedenen Dingen arbeiten könnten.

    
Peter Boughton 18.09.2008 21:40
quelle
1

Es hängt davon ab, welche Art von Richtlinien für Quellcodeverwaltung und Software Sie haben. Wenn ein Entwickler Code einchecken kann, den er beibehalten möchte, aber nicht das endgültige Produkt ist, haben Sie eine andere Situation als einen Ort, an dem jeder Check-in zu einem Build führt, der abgestempelt und möglicherweise an die Produktion gesendet wird sehr fest verschlossen. Gibt es mehrere Check-outs, bei denen etwas wie Subversion oder GIT verglichen mit etwas wie Sourcesafe, das mehrere Auschecken verhindern kann.

Ein anderer Punkt in diesem Zusammenhang ist, was mit einer Rezension gemeint ist: Geht jemand über den Code eines anderen, oder ist es jemand, der den Rest des Teams zeigt: "So habe ich das Feature ABC implementiert?" oder nur die Leads? Das ist sehr unterschiedlich und hängt etwas mit dem Quellkontrollpunkt zusammen, was die Art der Arbeitsumgebung betrifft, wie die Dinge organisiert und strukturiert sind.

    
JB King 18.09.2008 21:50
quelle
1

Viele der bereits erwähnten Codeüberprüfungsrichtlinien sind gute Praktiken, im richtigen Kontext.

  • Verfügen Sie als Hauptfunktion über einen Zweig, an dem Sie für alle Feature-Checkins arbeiten. Wenn die Funktion bereit ist, in main gebracht zu werden, überprüfen Sie die kombinierten Änderungen. Feature-Zweige sollten regelmäßige Zusammenführungen von main nehmen, damit die Verzweigung nicht zu stark divergiert und die Überprüfung vereinfacht wird.
  • Für checkins, die direkt in main gehen, ist ein Code-Review pro Checkin im Allgemeinen angemessen. Jeder Build-Break in main hat großen Einfluss auf alle Entwickler, die von main aus arbeiten.
  • Während der letzten Woche (n), bevor das Produkt ausgeliefert wird, sollten die Änderungen nicht nur überprüft werden, sondern auch nur von ernannten Entwicklern überprüft werden, um das Änderungsrisiko selbst zu senken mehr.

Bei meiner derzeitigen Firma erlauben wir gelegentlich Nachprüfungen, wenn (a) der Einreicher ein Senior-Entwickler ist und (b) eine Überprüfung wartet, die sich wesentlich auf andere auswirken würde. Zum Beispiel macht jemand eine Reparatur um 22 Uhr. Warten bis zum Morgen vor dem Einchecken kann sich auf andere Entwickler auswirken, ebenso wie auf automatisierte Tests und QA selbst. Wir geben einen Fehler / eine Aufgabenaufgabe in einen anderen Entwickler ein, damit sie die Überprüfung durchführen können. So wird die Kritik nicht vergessen.

    
Creg 18.09.2008 22:03
quelle
1

Lassen Sie den Code so bald und so oft wie möglich einchecken. Je früher die Codeüberprüfung zur Zeit des Codewechsels durchgeführt wird, desto besser. Lassen Sie die Qualitätskontrolle nicht zu und reduzieren Sie die Stallproduktivität. Code-Überprüfung wurde für den Entwickler vorgenommen, nicht der Entwickler für die Code-Überprüfung ...

    
mbrevoort 18.09.2008 22:14
quelle
0

Dies hängt wahrscheinlich von der Art der Überprüfung ab. Wenn Sie von einer formellen Inspektion sprechen, die Stunden oder Tage dauern könnte, würde ich argumentieren, dass das Einchecken in Ordnung ist. Wenn Ihr Shop mehr informelle Reviews / Walk-Throughs verwendet, machen Sie diese so einfach wie möglich und erzwingen, dass der gesamte Code vor dem Check-in überprüft wird.

Wenn Ihre Bewertungen einen "erhöhten Overhead" mit sich bringen, dann haben Sie recht, Sie möchten vielleicht vor der Überprüfung keine Überprüfung durchsetzen. Wenn Ihre Rezension eine informellere Angelegenheit ist, dann würde ich für die Durchsetzung einer Überprüfung-vor-Einchecken-Politik eintreten.

    
ahockley 18.09.2008 21:31
quelle
0

Eine andere Sache, die man beachten sollte, ist die Paarprogrammierung. Dadurch erhalten Sie noch mehr direktes Feedback. Sie überprüfen den Code, während Sie ihn schreiben. Dies ist einer der größten Vorteile.

Natürlich, wenn Sie dies nicht tun möchten, würde ich vorschlagen, eine Überprüfung vor dem Einchecken Politik. Wenn Ihre Leute so schlecht sind, dass Sie das Gefühl haben, dass Sie das tun müssen, können einige Nachhilfestunden in Ordnung sein.

Ich tendiere dazu, gelegentlich in meiner Ausfallzeit zu rezensieren und wenn ich über Code zurückgehe, um Dinge zu reparieren / ändern.

    
Aaron Jensen 18.09.2008 21:31
quelle
0

Ich denke, der beste Weg, um den Code-Review-Overhead vor jedem Check-in loszuwerden, besteht darin, die Entwickler dazu zu bringen, paarweise zu programmieren. Außerdem können Sie ein Teammitglied als offiziellen Reviewer für ein X-spezifisches Projektmodul bestimmen. Und dieser Typ kann täglich tiefere Kritiken schreiben.

    
Lucas S. 18.09.2008 21:33
quelle
0

Wir halten ein wöchentliches Mittagessenstreffen ab, um den Code zu überprüfen, den wir die Woche zuvor geschrieben haben. Wir versuchen im Allgemeinen, Code, der als Teil einer Iteration geändert wurde, zu überprüfen, bevor diese Iteration abgeschlossen ist. Auf diese Weise hat der Entwickler einige Tage Zeit, um die empfohlenen Änderungen vorzunehmen, bevor die endgültige Funktionalität bereitgestellt wird.

Da wir die Richtlinien regelmäßig (einmal pro Woche) überprüfen, haben wir festgestellt, dass die Änderungen, die vorgenommen werden müssen, im Allgemeinen gering sind und relativ schnell umgesetzt werden können.

ZUSATZ: Wir verwenden FxCop, um den Code bei jedem Build automatisch zu überprüfen. Unsere Reviews sind eher auf der weichen Ebene um Ansatz, Framework-Nutzung, Struktur, etc.

    
Tom Carr 18.09.2008 21:35
quelle
0

Jedes Mal, wenn Sie eine Funktion abschließen (stellen Sie sicher, dass Sie die Zeit dafür als eine der Aufgaben der Funktion festlegen!).

Code review Jeder Check-in ist sehr schlecht - ich möchte jede Stunde oder noch mehr einchecken, das ist unmöglich mit konstanten Code-Reviews. Darüber hinaus ist es unnötig - Ihr Hauptanliegen beim Einchecken ist es, sicherzustellen, dass Sie keinen vorhandenen Code brechen und in 90% der Fälle brauchen Sie dafür keine CR.

Wenn Sie an bestehendem Code arbeiten, den Sie nicht beherrschen, können Sie die Leute vor dem Einchecken für eine schnelle, dreitägige Überprüfung der empfindlichen Teile anrufen. Dies ist besonders wichtig in Projekten mit weniger als optimaler Codeabdeckung durch Tests (in einem solchen Projekt, selbst wenn alle Unit- / Integrationstests bestanden werden, wissen Sie immer noch nicht, ob Ihr Code gut ist).

    
ripper234 18.09.2008 21:37
quelle
0

Wenn Sie die Versionskontrolle so einrichten, dass Diffs an alle Entwickler gesendet werden, besteht die Möglichkeit, dass sie die "kleinen Überprüfungen" halbautomatisch durchführen.

  

Persönlich bevorzuge ich den Ansatz von   durchgehende Integration in   fange jeden, der den Build bricht, und   Führe dann Code-Reviews auf einem mehr durch   grobkörnige Basis am Ende eines   wichtiges Stück Arbeit. Ich nicht   Mandat Code Reviews für Moll   Refactorings.

Meine Gefühle sind die gleichen. Code Review ist für mich eine Qualitätsprüfung und Lernerfahrung über eine bestimmte Code-Einheit, nicht die graduellen Änderungen. Das Lernen von anderen Teilen ist imho so wichtig wie die anderen Vorteile der Code Review; jede Rezension, die ich gemacht habe, von der ich gelernt habe.

    
akauppi 18.09.2008 21:38
quelle
0

Die Paarprogrammierung ist nicht immer möglich oder wünschenswert; Menschen brauchen Zeit zum Denken und Arbeiten ohne Unterbrechung. Darüber hinaus führt die Überprüfung jedes Commits anstelle von größeren Changesets zu ginormous Commits, die schwer zu handhaben und zu verstehen sind (weniger commits = weniger reviews == Entwickler verbringen weniger Zeit in Meetings).

Sie wollen Entwickler oft committen. Es ist nicht unvernünftig, darauf zu bestehen, dass Commits den Build nicht unterbrechen, aber Sie wollen viele kleine logische Progressionen von Commits als Feature-Entwickler. Ein verteiltes Versionskontrollsystem kann hier helfen.

In einem kleinen Team mit klarem Dienstalter habe ich 30 Minuten am Tag damit verbracht, Diffs aus der Quellcodeverwaltung zu lesen, um ziemlich effektiv zu sein. Es ist hier in meinem Blog beschrieben.

Andernfalls benötigen Sie einige Werkzeugunterstützung (z. B. Crucible). Besprechungen zu planen und am Projektor zu gehen ist zu schmerzhaft; Du wirst damit aufhören.

    
davetron5000 18.09.2008 21:39
quelle
0

Ich stimme vorher. Failing schneller = Fehler billiger.

Das Problem mit dem Warten, bis Sie die Dinge reparieren, ist, dass es nur für kleine Teams funktioniert. Wenn Sie große Teams haben, steigt die Wahrscheinlichkeit, dass jemand an einem bestimmten Tag etwas kaputt macht (denken Sie an ein 100-köpfiges Team). Eine Build-Breaking-Defect-Rate von 0,5 pro Entwickler und Jahr ist für den Build durchschnittlich , JEDE WOCHE).

Auch im Hinblick auf die Aufrechterhaltung einer hohen Codequalität ist die beste Zeit, um die Codequalität zu verbessern, wenn sie am flexibelsten ist, bevor sie eingecheckt wird.

    
Wedge 18.09.2008 21:39
quelle
0

Code-Überprüfungen sind kein sehr guter Weg, Build-Breaks zu verhindern. Ein Einzelschritt-Build-Prozess, der bei allen Einträgen (Dev- und Build-Maschinen gleichermaßen) gleich ist, sollte die meisten Build-Breaks verhindern. Regelmäßige Tests (sowohl automatisiert als auch manuell) sind ein besserer Weg, um die meisten Fehler zu finden.

Code-Überprüfungen sind nützlich, weil:

1) Sie garantieren, dass jede Codezeile von mindestens zwei Augenpaaren gesehen wurde, falls das erste Paar in Urlaub geht oder das Team verlässt.

2) Sie erleichtern den Austausch von Ideen zwischen dem Code-Reviewer und dem Code-Reviewee.

3) Sie bieten eine zusätzliche Ebene der Rechenschaftspflicht.

Das Finden von Fehlern während einer Code-Überprüfung (ganz zu schweigen von Build-Breaks) ist ein schöner Bonus, aber nicht so üblich wie ich es bisher erlebt habe.

Der Vorteil von Code-Überprüfungen vor dem Einchecken besteht darin, dass Sie Ihre Code-Fehler (oder Build-Pausen) nicht an den Rest des Teams weitergeben. Es ist viel billiger (für das Team als Ganzes), so viele Probleme vor dem Check-in wie möglich zu erkennen. Je größer das Team, desto ausgeprägter dieser Effekt.

    
C. Dragon 76 18.09.2008 21:51
quelle
0

Code-Überprüfungen bei jedem Check-in reduzieren die Check-in-Anzahl und die Größe des Check-in-Volumens. Ein solcher großer Check-in erhöht das Risiko einer Unterbrechung.

Eine Code-Überprüfung kann eine Unterbrechung nicht verhindern. Wenn jemand denkt, dass der Code in Ordnung ist, kann ein anderer auch sehr schnell denken. Eine kontinuierliche Integration mit einer hohen Codeabdeckung ist besser. Das Schreiben eines Testfalls für die meisten Checkins macht mehr Sinn.

    
Horcrux7 18.09.2008 21:52
quelle