Der perfekte Workflow für Pull-Requests (als Werkzeug für Code-Reviews)?

gistfile1.txt

Pull-Requests funktionieren wunderbar für kleinere Sachen: Bugfixes, kleinere Features. 
Alles, wo die Anzahl der Changes überschaubar bleibt und sich ein Review in kurzer Zeit
gut bewerkstelligen lässt. Siehe: http://blog.ploeh.dk/2015/01/15/10-tips-for-better-pull-requests/

Die Frage ist, wie man das bei größeren Features organisiert. 

Angenommen ein Entwickler bekommt die Aufgabe, ein komplett neues Modul in einer App 
zu bauen. Bricht man die Aufgabe – was für die Reviews wichtig ist – in kleine 
Einheiten auf, hilft das sicherlich auch schon mal allgemein bei der Strukturierung.

Es ergeben sich also nun, beispielhaft, zwei Tickets: View Model bauen, View bauen.

Er erzeugt abgehend von `develop` einen Branch für das View Model und reicht den 
als PR ein. Nun kann er die View nicht ohne View Model bauen. Was also hier tun?

1) Warten, bis jemand das Review durchführt?

> Blöd, wenn es Freitagnachmittag und er gerade im Flow ist. Was für eine Verschwendung.

2) Zurück zu develop und weiterbauen?

> Geht nicht, er braucht den letzten Code-Stand.

3) Nicht von `develop`, sondern vom View Model-Branch abzweigen und den View-Branch
   aufmachen?
   
Das würde technisch gehen, birgt aber das Risiko, dass der erste PR abgelehnt oder
noch mal massiv geändert werden muss. (Gut, hier kann er die Änderungen nachher
rübermergen).

- Empfehlungen?

Das machen wir so, aber je nach Umfang des Features, wird der PR halt groß und umfangreich, was das Reviewen erschwert.

Wir nutzen GitFlow. Jeder arbeitet in seinem Feature Branch. Ist der fertig, wird ein PR erstellt egal wie groß der wird.
Da die meisten PRs in nur 5 Minuten fürs Review benötigen, macht es nicht viel aus, wenn mal welche dabei sind, für die man 30 Minuten oder länger braucht. Das ist eh eher selten. Es wird meist darauf geachtet, dass die Changes klein sind (klar, geht nicht immer). Zur Not wird aber auch mal ein Feature kleiner gemacht.
Ein Paar regeln gibt es halt auch dazu:

• Jeder Reviewed jeden.
• Kein PR darf älter als 24 Stunden sein
• Ein Approval reicht, außer es werden vom Autor mehr gefordert

Das funktioniert im Großen und Ganzen sehr, sehr gut und zuverlässig

@CayasSoftware Nichts anderes machen wir. Aber das Ergebnis wird mir zu groß.

Ein paar benimmse Tippsvon @JuergenGutsch Ja, dass gefällt mir. Und BTW @forki hat natürlich recht! PR + CR hat natürlich die wichtige Aufgabe für ein paar Stunden innezuhalten und die eigene Arbeit zu kommunizieren. Deshalb auch keine meterlangen PRs, oder zu viele offene PRs. Jeder im Team ist gefragt, den anderen schnell und unkompliziert zu unterstützen.

Da bin ich dabei:

• Jeder Reviewed jeden.
• !!!Kein PR darf älter als 24 Stunden sein!!!
• Ein Approval reicht, außer es werden vom Autor mehr gefordert

@MikeBild Du arbeitest am Wochenende? Get a life! ;-)

Wir werden jetzt erst mal Folgendes machen:

• PRs bleiben so klein wie möglich
• Reviews erfolgen kontinuierlich (Bekanntmachung im Team über einen Slack-Channel (da man in GitLab nur einen Reviewer auswählen kann :()
• Sollte ein Review nicht so schnell möglich sein, wie man weitermachen möchte, wird vom Feature-Branch der nächste abgezweigt, so dass es am Ende zwei PRs gibt. Was passiert, wenn es hier mal zu Komplikationen wg. Problemen im vorangegangenen PR kommt, warten wir einfach ab.

Hallo die Herren,

ich habe nicht sämtliche Kommentare gelesen, will aber auch noch meinen Senf dazu geben: @aspnetde's ursprüngliche Frage scheint aus meiner nicht ganz zu seinen Schlüssen (https://gist.github.com/aspnetde/511c72e743fb658f71f186df12fca85d#gistcomment-1886919) zu passen ;)

Ich denke das Problem "zu großer" PRs lässt sich ganz leicht umgehen, indem man einen PR (und damit topic-Branch) für jedes Produktinkrement erzeugt. Das ist an sich gut validierbar und weil Inkremente immer Durchstiche sind damit gut per UI/API/sonstwas zu verifizieren. Will der Developer am nächsten Inkrement weiterarbeiten während er auf das Review des PR wartet erzeugt er einen neuen topic-Branch am letzten Commit des PR. Muss der PR nachgebessert werden, erfolgt ein rebase des zweiten Arbeitsgangs gegen die Korrekturen.

Wer öfter mit solchen Abhängkeiten zwischen Branches zu tun hat dem sei dieses hier empfohlen: https://github.com/greenrd/topgit

@agross Na das passt doch. Mit dem Rebase bleibt die Timeline (bei 150 commits ;-)) auch verständlich(er). Schön das jemand gleich ein Konzept, Methode und Tooling anbietet. Wir machen das -ähnlich- bisher manuell, um niemanden zu schocken. Danke! Ich schau mir das mal an.

@agross Ich brauchte irgendwas, das einen linearen Entwicklungsprozess veranschaulicht. Mir fiel dann die hypothetische test-getriebene Entwicklung des View Models ein, mit nachgelagerter Erstellung der View. Nicht, dass das irgendwie praxisnah wäre. 🙄

Aber am Ende sind wir alle beisammen.

Danke für den Austausch an die Runde 👍