Wie kann ein Code am besten überprüft werden, bevor er an den Trunk übergeben wird? (SVN)

14

Wie kann ein Code am besten überprüft werden, bevor er an den SVN-Trunk übergeben wird? Eine Idee, über die ich nachdenke, besteht darin, den Entwickler dazu zu bringen, seinen Code an einen Zweig zu übergeben und dann seinen Code zu überprüfen, während die Zweigrevisionen in den Stamm eingefügt werden. Ist das eine gute Übung? Wenn nicht, was kann noch getan werden, um einen Code zu überprüfen, bevor er an den Trunk übergeben wird?

Meysam
quelle
2
Sie könnten sich Tools wie Crucible ansehen , die die Überprüfung vor dem Festschreiben unterstützen.
Gyan aka Gary Buyn
3
Gibt es einen Grund, einen Code nicht zu überprüfen, nachdem er an den Trunk übergeben wurde?
Mücke
3
@gnat Nun, ich denke, es ist besser, dass Junior-Entwickler ihren Code woanders festschreiben und diese Änderungen dann von einem Senior-Entwickler in den Trunk einbinden, nachdem er sie überprüft und sichergestellt hat, dass diese Änderungen in Ordnung sind, um in den Trunk zu gelangen. Sie wissen, dass ein leitender Entwickler nach Durchsicht des Codes, der direkt in den Trunk geschrieben wurde, möglicherweise entscheidet, dass dieser Code Probleme hat, die rückgängig gemacht werden sollten. Wir hätten verhindern können, dass dieser problematische Code überhaupt in den Trunk geschrieben wird. Das ist die ganze Idee.
Meysam
Haben Sie es andersherum versucht oder raten Sie einfach?
Mücke

Antworten:

12

Es gibt jedoch zwei Schulen - das, was Sie vorschlagen oder "vor dem Festschreiben überprüfen". Die meisten Unterschiede können als negative und / oder positive gesehen werden. - z. B. keine Nachverfolgung von Änderungen, die durch eine Überprüfung verursacht wurden - möchten Sie diese als diskrete Commits betrachten oder sind Sie nur an der endgültigen Arbeit interessiert?

Vor dem Festschreiben überprüfen - es ist keine Verzweigung erforderlich (kann jedoch auf Wunsch durchgeführt werden), sodass Überprüfer auf Arbeitsordner zugreifen können. Der Code kann während und nach der Überprüfung ohne Nachverfolgung geändert werden. Durch die Überprüfung verursachte Korrekturen werden nicht im Repository angezeigt.

Überprüfung nach Festschreibung (für einen Zweig) - Für jede Überprüfung muss ein Zweig gedreht werden (obwohl dies möglicherweise bereits im Workflow enthalten ist). Der zur Überprüfung eingereichte Code kann nicht geändert werden, ohne die Änderungen nachzuverfolgen. Jemand muss die überprüften Zweige zusammenführen und nachverfolgen, was überprüft wurde und was nicht.

Dies hängt weitgehend von der Teamkultur und -erfahrung ab. Was ist Ihr Vertrauensmodell, was ist das Hauptziel für Reviews? Ich persönlich bevorzuge die Überprüfung nach dem Festschreiben, da Änderungen aufgrund der Überprüfung nachverfolgt werden können. Wir verwenden jetzt Git und Gerrit, da sie eine gute Balance zwischen den beiden Optionen bieten.

mattnz
quelle
1
Die konstante Bildung von Zweigen und das wiederholte Zusammenführen sind eine Beeinträchtigung, die potenziell (und unwiderruflich) umweltschädlichen Stamm bei weitem überwiegt. Im Allgemeinen lautet die Hauptanweisung für die Versionskontrolle "Don't break the build". Wenn Sie dies tun können, kann das Einchecken nicht wirklich schaden, und alles andere ist nur eine nachträgliche Anpassung.
Spencer Kormos
Die Überprüfung nach dem Festschreiben eines Zweigs eignet sich gut für die Verwendung von Feature-Zweigen: Sie starten einen neuen Zweig für jedes neue Feature oder jede neue Fehlerbehebung. Wenn es fertig ist und die Überprüfung bestanden hat, wird es zum Trunk zusammengeführt. Auf diese Weise enthält der Trunk nur vollständige, überprüfte Änderungen. Da Feature-Zweige von kurzer Dauer sind, sind die Zusammenführungen im Allgemeinen trivial. Der andere Vorteil ist, dass der Kofferraum nur vollständige Funktionen und Fehlerbehebungen enthält - alles, was halb gebacken ist, existiert nur auf einem Zweig.
Stephen C. Steel
7
  1. Führen Sie vor dem Festschreiben 'svn diff' aus, um eine Patch-Datei zu generieren.
  2. Senden Sie die Patchdatei an den Prüfer, der sie auf eine saubere Kopie des Trunks anwendet.
  3. Der Prüfer überprüft Änderungen mit dem Unterschieds-Viewer seiner Wahl.
  4. Der Prüfer führt Builds durch und führt Tests durch.
  5. Wenn alles gut geht, teilen Sie dem Entwickler mit, dass er die Änderungen überprüfen kann. Bei
    Problemen kehrt der Entwickler zu Schritt 1 zurück.
Charles E. Grant
quelle
5

Es gibt die ideale Welt und dann die reale Welt.

In der idealen Welt wird Ihr gesamter Code getestet, sodass Sie sicher sein können, dass alles, was eingecheckt wird, funktioniert oder Sie wissen, dass es kaputt ist, weil es einen oder mehrere Tests nicht besteht. Außerdem wird jeder, der nicht so erfahren ist, mit jemandem gepaart, der Erfahrung hat. Daher erfolgt die Codeüberprüfung im laufenden Betrieb (Sie verpflichten sich, während Sie fortfahren).

In der realen Welt liegen die Dinge anders. Das Geschäft will diese Veränderung jetzt liveund wird Ihnen mit einem vollkommen geraden Gesicht sagen, dass Sie ja Zeit haben, den Code zu bereinigen und später Testfälle hinzuzufügen. Sie werden wahrscheinlich keine Zeit haben, alles zu überprüfen, und der Prozentsatz des Codes, der durch Tests abgedeckt wird, nimmt ständig ab. Der Hauptgrund für die Codeüberprüfung besteht darin, dass Junior-Entwickler von Senior-Entwicklern lernen (wenn Zeit dafür ist), indem eine erfahrenere Person Änderungen überprüft und "bessere Methoden für die Ausführung von Dingen (TM)" vorschlägt. Sie werden leitende Entwickler haben, die nur ungeprüften Code schreiben. Nur für eine Codeüberprüfung zu verzweigen und dann zusammenzuführen, ist eine enorme Zeitverschwendung. Eine Möglichkeit, dieses Problem zu lösen, besteht darin, eine regelmäßige zweistündige Teambesprechung zu organisieren, bei der Sie ein oder zwei Änderungen auswählen, an denen die Mitarbeiter kurzfristig gearbeitet haben, und ihre Autoren zum "Präsentieren" auffordern. Ihr Ansatz durch gemeinsames Betrachten des Codes auf einem Projektor oder Ähnlichem Dies kann zu interessanten Diskussionen führen (die normalerweise ein wenig vom Thema abweichen), verbessert jedoch im Allgemeinen das Verständnis für die richtige Vorgehensweise. Und der Druck, möglicherweise Ihren Code vorlegen zu müssen, lässt manche Leute es besser machen ;-)

Oder Sie haben Glück und können in einer realen Umgebung arbeiten, in der es nicht so hektisch ist. Programmierer werden für ihre Taten geschätzt, anstatt sie zu missbrauchen, und es bleibt Zeit, alles richtig zu machen. In diesem Fall lautet meine Antwort: Probieren Sie einige der verschiedenen Methoden aus, die in den Antworten vorgeschlagen werden, und finden Sie heraus, welche für Ihr Team und Ihre Arbeitsweise am besten geeignet ist.

Amos M. Carpenter
quelle
+1 für die wöchentliche Bewertungsidee. Vielleicht muss ich es versuchen
Jamie Taylor
@JamieTaylor: Nun, es ist ein Kompromiss. Wenn Sie (und Ihr Entwicklerteam) die Zeit haben, würde ich stattdessen vollständige Codeüberprüfungen empfehlen. Aber es ist eine gute Möglichkeit, Wissen im Team zu teilen.
Amos M. Carpenter
2

Die Filialen sollten auf der Grundlage meiner Erfahrung mit der Verwendung in Pre-Commit-Überprüfungen bei früheren Jobs in Ordnung funktionieren.

Beachten Sie, dass wir vor dem Festschreiben nur Überprüfungen für kritische Patches für den Serienversionskandidatencode verwendet haben, sodass es nicht viele Zweige gab (routinemäßige Änderungen wurden nach dem Festschreiben überprüft).

Da Sie anscheinend Pre-Commit-Überprüfungen für alle Änderungen verwenden werden, müssen Sie möglicherweise eine große Anzahl von Filialen verwalten. Wenn Sie erwarten, dass der Entwickler durchschnittlich eine "überprüfbare" Änderung pro Woche vornimmt, stehen Ihnen pro Entwickler im Team jährlich etwa 50 Niederlassungen zur Verfügung. Wenn Sie kleinere Arbeitsstücke verwenden - wie diejenigen, die 1, 2, 3 ... Tage benötigen - multiplizieren Sie 50 mit 2, 3, 5 ... entsprechend.

Im Folgenden finden Sie einige weitere Überlegungen, die Sie berücksichtigen sollten, wenn Sie dies am besten möchten .

1. Behandlung von Fällen, in denen eine verzögerte Überprüfung den Code blockiert, der für andere Teammitglieder benötigt wird

Feststellen, Überwachen und Lösen von Konflikten im Zusammenhang mit Fristen für die Codeüberprüfung. Nach meiner Erinnerung an Überprüfungen im Vorfeld von Routineänderungen, die ich in einem der vergangenen Projekte durchgeführt habe, beträgt die angemessene Frist etwa 3 Tage, und es ist Zeit, sich Sorgen zu machen, wenn die Überprüfung nicht mehr als 1 Tag nach der Einreichung abgeschlossen ist.

Zum Vergleich: Bei Überprüfungen nach dem Festschreiben sind diese Anforderungen viel entspannter (ich habe eine Frist von 2 Wochen und mache mir nach 1 Woche Sorgen). Da Sie jedoch Überprüfungen vor dem Festschreiben als Ziel festlegen, ist dies wahrscheinlich nicht interessant.

2. Zusammenführen von Konflikten beim Senden von überprüftem Code

Was ist zu tun, wenn das Festschreiben für überprüften Code durch widersprüchliche Änderungen blockiert wird, die von einer anderen Person festgeschrieben wurden, während der Code auf die Überprüfung wartete?

Einige Optionen zu prüfen sind

  • Rollback zum Anfang und Aufforderung an Entwickler, die Änderung erneut zu implementieren und zu überprüfen
    In diesem Fall müssen Sie möglicherweise negative Auswirkungen auf die Teammoral haben (wird!).
  • Übergabe der Verantwortung für das Zusammenführen an ein anderes Teammitglied ("Zusammenführungsmaster")
    In diesem Fall müssen Sie auch entscheiden, ob Zusammenführungen per se vor dem Festschreiben überprüft werden sollen oder nicht - und wenn ja, was tun, wenn Diese Verschmelzung trifft wiederum auf einen anderen Konflikt.
  • Ignorieren Sie Änderungen an überprüftem Code in der Phase des Zusammenführens.
    In diesem Fall müssen Sie möglicherweise einen negativen Einfluss auf die Teammoral ausüben, der sich aus der Tatsache ergibt, dass der festgeschriebene Code von dem Code abweicht, der überprüft wurde.
  • Erfinden Sie einen Weg, um Konflikte zu vermeiden. Ein einfacher
    Ansatz besteht darin, nur einem Entwickler zu erlauben, bestimmte Dateigruppen zu ändern. Dies schützt Sie jedoch nicht vor Änderungen, die Dateien nicht direkt ändern, sondern sie durch interne API-Änderungen beeinflussen . Möglicherweise stellen Sie auch fest, dass diese Art von "pessimistischem Sperren" systemweite Änderungen und tiefgreifendes Refactoring ziemlich mühsam macht.

Zum Vergleich gibt es in Überprüfungen nach dem Festschreiben keine Probleme dieser Art (da es sich um Code handelt, der bereits per Definition zusammengeführt wurde). Da Sie jedoch Überprüfungen vor dem Festschreiben als Ziel festlegen, ist dies wahrscheinlich nicht interessant.

3. Laden Sie den Entwickler, der auf eine Überprüfung wartet

Legen Sie eine explizite Richtlinie fest, ob der Entwickler, der die Überprüfung eingereicht hat, zu einer neuen Aufgabe wechseln oder etwas anderes tun soll (z. B. dem Überprüfer nachjagen).

Zum Vergleich benötigen Überprüfungen nach dem Festschreiben kaum explizite Richtlinien (da es selbstverständlich ist, nach dem Festschreiben des Codes mit der nächsten Aufgabe fortzufahren und die Frist für die Überprüfung ein oder zwei Wochen zu berücksichtigen) nicht interessant.

Mücke
quelle
0

Jede Entwicklung, die einer Überprüfung bedarf, müsste sich in einem separaten Zweig befinden. Die Verzweigung sollte also bereits vorhanden sein, bevor die Zeit für eine Überprüfung kommt. Dann müsste der Schritt sein:

  1. Rezension
  2. Überarbeiten (möglicherweise)
  3. zurück zur Übersicht (möglicherweise)
  4. In Kofferraum einbinden

Verschmelzung ist das schwierige Stück. Je länger der Zweig unabhängig bleibt, desto schwieriger wird es, ihn wieder in den Stamm zu integrieren. (Möglicherweise ist es auch schwieriger zu testen.)

Cross-Merging ist eine mögliche Lösung. Führen Sie vor dem Zusammenführen in den Stamm (Schritt 4 oder noch früher, z. B. vor Schritt 3 oder Schritt 1) ​​den Stamm in den Zweig zusammen. Der Entwickler kann es tun und testen. Dann holt der Zweig den Stamm ein und es wird einfacher, ihn mit dem Stamm zusammenzuführen. Das Zusammenführen in den Kofferraum wird am besten von Ihnen oder demjenigen durchgeführt, der für den Kofferraum zuständig ist.

Einige Leute versuchen, ein Rebase anstatt ein Cross-Merge durchzuführen. Einige Leute argumentieren, dass Rebase böse ist. Das ist eine andere Debatte.

Uday Reddy
quelle