Alle Folgen

Webcafé — Folge 7

Merge Requests

Heute sprechen wir über den zentralen Grundpfeiler unserer Qualitätssicherung und des Wissenstransfers: Merge Requests.

hören lesen

Folge 7 Merge Requests 51 min · 16 Kapitel
0:00 51:01

Am Mikrofon

01 — Worum geht es

Worum geht es?

Heute sprechen wir über den zentralen Grundpfeiler unserer Qualitätssicherung und des Wissenstransfers: Merge Requests.
Wir gehen Schritt für Schritt durch die unterschiedlichen Stufen, die wir bei den Merge Requests in unserem Unternehmen etabliert haben. Von dem Erstellen bis hin zum ausführlichen Review.

02 — Transkript

Das Gespräch, Wort für Wort

Kapitel

8.944 Wörter in 16 Abschnitten. Jede Zeitmarke springt an die passende Stelle im Audio. Automatisch transkribiert und maschinell nachkorrigiert — im Zweifel gilt das Gesprochene.

Begrüßung 0:00–1:45

  1. 0:00

    Hallo und herzlich willkommen zu einer weiteren Folge von unserem Webcafé.

  2. 0:08

    Heute bin ich auch wieder im Studio, in unserem virtuellen Studio mit dem Kay. Hallo, das bin ich. Ja, hallo, hallo. Und wir haben uns heute ein ganz schönes Thema vorgenommen, auf das wir in Folge 1 schon mehrfach zu sprechen gekommen sind. Das ist ein zentrales Thema in unserer Qualitätssicherung. Da geht es um Merge Requests. Aber wie jede Woche, bevor wir durchstarten, müssen wir erstmal wissen, was wir für ein Getränk dabei haben, Kay. Ich dachte schon, du hättest das komplett übersprungen, weil ich habe nämlich jetzt heute extra mir hier gerade einen Cappuccino bringen lassen und mir diesmal extra aufgeschrieben, dass das nämlich eine Espresso-Bohne aus Tansania ist

  3. 0:45

    mit Noten von dunkler Schokolade, Mandeln und Zitrus. Das hört sich fantastisch an. Du gehst in letzter Zeit immer mehr so ein bisschen Richtung Kaffee, das macht mir ein bisschen Sorge. Ja, weil wir einfach früher aufnehmen und da kann ich meinen Morgenkaffee noch einschieben. Ja, stimmt. Ich habe mich heute auch ein bisschen gequält, mir schon einen Schwasttee zu machen so früh. Ich habe nämlich ein Twinings dabei. Das ist eine sehr, sehr alte englische Teemarke, ist auch ein English Breakfast-Tee. Ich glaube, die sind von 1706 oder sowas um den Dreh. Also wirklich sehr alt. Ist von einem Mitarbeiter der Ostindien-Kompanie gegründet worden.

  4. 1:19

    Aber das führt jetzt so sehr ins Detail. Ich bin mir noch nicht ganz sicher, ob es mein Lieblings-English-Breakfast-Tee wird. Den Tedlay, den ich glaube ich schon mal in einer anderen Folge dabei hatte, der gefällt mir doch nach wie vor auch extrem gut. Dass du alleine innerhalb der Kategorie der English-Breakfast-Tees schon Favoriten nennen kannst, finde ich sehr beeindruckend. Ja, man kommt so rein ins Tee-Game. Ja, anscheinend. Also ich habe immer nur Beutel in die Tasse getan. Aber du bist da wohl noch mal ein bisschen tiefer drin. Heute haben wir das Thema Merge-Requests. Kay, ich würde das Wort direkt an dich übergeben.

Einleitung ins Thema 1:45–3:10

  1. 1:50

    Wobei, ich habe einen kleinen Einwurf. Ich habe mich nämlich selbst gefragt, was der Unterschied zwischen Pull und Merge-Request ist. Wir benutzen das immer synonym. Und dann habe ich mir im Vorhinein das noch ein bisschen durchgelesen, was da die Unterschiede sind. Ich glaube, Kay, dass du das weißt. Deswegen kann ich das ja einfach spoilern. Letztlich ist es tatsächlich das Gleiche. Also wir können es synonym benutzen. Und der wesentliche Unterschied ist, dass Plattformen wie GitHub und Bitbucket das Ganze eben Pull-Request nennen. GitLab zum Beispiel nennt das Merge-Request. Und ich habe gelesen, dass das so ein bisschen daherkommt, was man entweder als ersten oder letzten Schritt in dem PR, sage ich jetzt mal, macht.

  2. 2:28

    Bei GitHub sagen die, wir pullen als erstes einen Feature-Branch. Und bei GitLab sagen sie halt, der letzte Schritt im Merge-Request ist dann auch der Merge in den Main- oder Master-Branch.

  3. 2:41

    Und deswegen nennen die das so. Also wenn wir die Begriffe hier durcheinander würfeln, seid nicht verwirrt, sondern das ist am Ende das Gleiche. Das passiert mir auch immer wieder, das eine oder das andere. Wollen wir kurz darauf eingehen, was ein Merge-Request überhaupt ist? Oder trauen wir unseren Hörern zu, dass sie das schon wissen? Das ist eine spannende Frage. In den letzten Podcasts sind wir immer so ein bisschen über Fachthemen hinweg galoppiert und haben erwartet, dass die Leute das kennen. Aber vielleicht kannst du es aus dem Stand mal in einem Satz beschreiben. Aber ich hätte gedacht, dass du jetzt eine Definition machst.

Was ist ein Merge Request? 3:10–4:52

  1. 3:12

    Aber ich versuche es. Also eine Merge-Request für uns ist eben, wie ich schon gesagt habe, ein ganz wichtiges Instrument der Qualitätssicherung. Und letztlich geht es darum, dass wenn ein Entwickler, eine Entwicklerin neuen Code schreibt, dann haben wir natürlich eine Versionierung dahinter.

  2. 3:27

    Und ich mache mir also ein Feature-Branch, also ziehe mir letztlich eine Kopie von einem aktuellen Stand, meistens aus dem Main-Branch, also den Hauptcode, mache mir eine Kopie und arbeite in dieser Kopie.

  3. 3:39

    Und in dem Moment, wo ich mit meiner Arbeit fertig bin an dieser Kopie, wo ich eine Erweiterung zum Beispiel programmiert habe oder einen Bugfix, würde ich den Code wieder zurück committen, zurückmergen in den Hauptbranch.

  4. 3:53

    Und genau in dem Moment kommt eben unser Merge-Request ins Spiel. Wir haben nämlich den Ablauf, dass wir jemand anders, einen anderen Entwickler, eine andere Entwicklerin, dann eben einmal zum Review bitten und bitten, darüber zu gucken und zu gucken, ob alles in Ordnung ist.

  5. 4:09

    Und auf die Details gehen wir gleich noch ein bisschen ein. Und erst wenn jemand sein Okay gibt, kann dieses neue Feature, dieser neue Branch in den Main-Branch gemerged werden und ist dann tatsächlich live.

  6. 4:21

    Also dieser Prozess des Reviews, das ist letztlich dann der Merge-Request. Du hast es gerade gesagt mit um Review bitten, aber tatsächlich ist es ja so, dass wir schon eiskalt die Berechtigungen so haben, dass nur ein ausgesuchter Kreis von Personen diese Merges auf den Protected Main-Branch überhaupt erst vornehmen kann. Das heißt, den Kollegen bleibt gar keine andere Möglichkeit, wenn sie ihre Arbeit veröffentlicht sehen wollen, dass sie das über so einen Merge-Request machen, der dann auch gereviewt werden muss von einer Autoritätsperson, sag ich mal.

  7. 4:50

    Und zwar also fachliche Autorität. Und du hast das vorhin schon gesagt, aber das ist, glaube ich, die Grundlage für die ganze Qualität, die wir uns auf die Fahne geschrieben haben. Weil nur durch diese Merge-Requests können wir sicherstellen, dass die Qualitätskriterien wirklich auch sichergestellt werden können.

Merge Requests als Qualitätssicherung 4:52–6:32

  1. 5:08

    Zum einen automatisiert, aber auch zum anderen durch das handwerkliche Review von Leuten, die sich im Code auskennen.

  2. 5:16

    Das ist meistens ich, aber auch andere Kollegen, die so bestimmte Fachbereiche in den Anwendungen haben, wo sie sich besonders gut auskennen. Da ist das unerlässlich. Und es gibt auch die Sicherheit für die Kollegen, eben dadurch, dass sie mit dem Haupt-Branch, also dem Production-Branch, keinen Quatsch machen dürfen, sich auch frei auszuprobieren.

  3. 5:35

    Also man kann dann auch neue Leute da dransetzen, die überhaupt gar keine Erfahrung haben und die müssen keine Angst haben, entgegen zu so einer Live-Seite, wo man vielleicht per FTP drauf herumturend, dass sie irgendwas kaputt machen.

  4. 5:45

    Weil im Zweifel schmeißen wir einfach den Branch weg und machen es neu und es besteht überhaupt gar keine Gefahr. Das ist für mich ein ganz wichtiger Punkt. Bei uns ist es nämlich so, dass auch Praktikanten, Auszubildende, unerfahrene Programmierer und Programmiererinnen wirklich schon an Live-Projekten arbeiten, in den Main-Branch reinmergen können, in dem Moment, wo das eben gereviewt wurde.

  5. 6:07

    Und wir stellen durch diese Review-Prozesse, sicher durch diese Merge-Requests, dass am Ende bei allen Entwicklern und Entwicklerinnen die gleiche Qualität bei rauskommt, egal welche Erfahrungsstufe.

  6. 6:16

    Und das führt natürlich dazu, dass das bei einem unerfahrenen Entwickler dann zwei, drei Schleifen dreht und bei einer erfahrenen Entwicklerin dann vielleicht direkt beim ersten Mal durchgeht.

  7. 6:27

    Aber letztlich, das, was hinten rauskommt, ist immer unser Produkt höchster Qualität. So ist das, ganz genau. Ich würde jetzt einfach so ein bisschen dem Verlauf folgen, den tatsächlich so ein Code-Review auch bei uns im Unternehmen macht. Vielleicht als kurzen Überblick, dann gehen wir als erstes darauf ein, wie man so ein Merge-Request möglichst sinnvoll stellt. Dann, wie vielleicht mögliche Review-Schleifen vorab aussehen, bis es dann hin zum finalen Code-Review und Mergen geht.

Ablauf im Überblick 6:32–8:10

  1. 6:58

    Vielleicht vorab noch eine Besonderheit, die bei uns ist, gegenüber den ganzen anderen Erklärungen, die man so im Internet findet, wie man gute Merge-Requests macht.

  2. 7:07

    Unsere Merge-Requests sind immer, oder in 90 Prozent der Falle sind die Task-basiert. Das heißt, wir haben vorher in einem Projektmanagement-Tool eine Aufgabe beschrieben, die gemacht werden soll. Und diese Aufgabe ist Grundlage für diesen Merge-Request. Und das bedeutet zum Beispiel, dass in dem Merge-Request selber nicht mehr eine sonderlich fancy Beschreibung oder einen Titel benannt werden muss, der sagt, worum es geht,

  3. 7:33

    sondern nur eine Referenz auf die Task in jetzt zum Beispiel ClickUp oder Jira. Und da stehen die ganzen detaillierten Informationen drin. Was wir schon gerne ergänzen, ist natürlich trotzdem ein Screenshot von der tatsächlichen Umsetzung dann zum Beispiel. Wenn man im Task jetzt, wenn man im Frontend unterwegs ist, nur ein Mockup hat, dann würden wir schon kurz ein Screenshot oder sogar ein Video, ein kurzes GIF einbinden, wie das dann aussieht in der Umsetzung, weil das dem Reviewer, kann ich zumindest von mir sagen, immer extrem hilft, auch visuell zu sehen, was da passiert ist. Das geht natürlich bei Backend-Sachen etwas schlechter, aber eine kurze Beschreibung ist manchmal dann doch sinnvoll.

Schritt 1: MR stellen 8:10–11:08

  1. 8:11

    Genau, also vielleicht ganz konkret, jemand bekommt über irgendein Projektplanungstool eine Task zugewiesen und dann erstellt er einen, wir nennen es Feature-Branch oder eine Task-Branch basierend für diese eine Aufgabe vom Main ab.

  2. 8:24

    Also wir haben da kein sonderlich kompliziertes Git-Konstrukt, sondern einen neuen Branch vom Name. Den Branch benennt er irgendwie so, wie das mit der Aufgabe passt. Üblicherweise haben die ja eine ID oder sowas. Und dann entwickelt er fleißig darauf herum oder die Entwicklerin und stellt dann am Ende wieder einen Merge-Request gegen den Branch, aus dem es rausgezogen wurde. Also üblicherweise von Main in den Task-Branch, nenne ich es mal, um wieder zurück zu Main. Du hast es schon gesagt, so ein Bild oder sogar ein kleines Video oder so ein Screenshot ist super hilfreich in der Beschreibung,

  3. 8:59

    wenn die Aufgabe irgendwas enthält, was visuell präsentiert werden kann. Und das ist bei uns üblicherweise ja so ein UI in irgendeiner Webseite. Und das hilft allen Leuten, eben kurz ins Thema reinzukommen, nochmal schnell mit einem Blick zu sehen, was wurde denn hier gemacht und vielleicht auch schon so ein paar konzeptionelle Probleme festzustellen. Und das befreit aber natürlich nicht. Und das ist ein Riesenpunkt, den Branch selber mal auszuchecken und selber im Browser herumzuwerkeln und zu schauen, was hat denn der Kollege oder die Kollegin da gemacht. Ja klar, das Auschecken für den Reviewer ist eigentlich Pflicht bei uns.

  4. 9:38

    Und ich muss das eigentlich streichen, sondern es ist Pflicht. Aber es gibt natürlich trotzdem auch PRs, wo es rein logisch nicht so wahnsinnig viel Sinn macht, den jetzt auszuchecken, gerade bei kleineren Bugfixes, wenn dann nur irgendein Typo korrigiert wurde. Dann muss er das natürlich nicht auschecken. Genau, aber ich möchte jetzt gar nicht so sehr springen, weil wir sind jetzt noch bei dem Schritt, wo der Kollege oder die Kollegin den Merge-Request stellt, wenn ich dich da so unterbrechen darf. Absolut. Wir haben schon genau kurz über so einen Screenshot gesprochen. Was auch hilfreich ist, ist, wenn es bei der Bearbeitung der Aufgabe irgendwelche Besonderheiten gab,

  5. 10:11

    dass man die in der Beschreibung benennt oder vielleicht sogar direkt selber einen Kommentar im Code direkt macht.

  6. 10:19

    Weil diese Merge-Requests können bei uns auch per Berechtigungen, im GitLab machen wir das übrigens, auch nur gemerged werden, wenn alle Threads resolved sind, sagt man, also alle Kommentare erledigt. Und wenn man dann als Entwickler irgendeinen Punkt hat, über den man gerne sprechen möchte, weil da noch irgendwas unklar ist, dann halten wir die Kollegen und die Kollegin auch an, das schon direkt selber in ihrem eigenen Review zu kommentieren, damit das gar nicht verloren geht.

  7. 10:46

    Möglicherweise hat man ja so eine Stelle, wo man sagt, ah, hier bin ich mir nicht so ganz sicher, ob das so das richtige Vorgehen ist, oder ich fühle mich an dieser Stelle selber noch nicht so ganz wohl. Normalerweise sprechen die das natürlich an oder ab, bevor der Merge-Request gestellt wird, aber manchmal ist das eben auch eine Möglichkeit, im Merge-Request zu sagen, hey, hier ist eine Stelle, da ist irgendwas besonders, lass uns dann nochmal drüber sprechen. Eine spannende Frage ist, was für ein Scope hat denn so ein Merge-Request? Weil einerseits ist es natürlich sinnvoll, wenn der Merge-Request nur die Änderungen enthält,

Was gehört in einen MR? 11:08–18:03

  1. 11:21

    die auch tatsächlich unbedingt für diese Aufgabe notwendig sind. Das heißt, so kompakt und so klein wie möglich und gerne auch mehrere einzelne Merge-Requests, wenn sich das irgendwie ergibt. Wir hatten jetzt letzten zum Beispiel einen Fall, wo aus Architekturgründen irgendeine Variable umbenannt werden sollte und das war eine relativ zentrale Variable und die hatte Abhängigkeiten auf 100 Dateien oder so,

  2. 11:48

    die geändert wurden, aber durch das Refactoring in dem PhpStorm-IDI, die wir benutzen, macht das quasi keine Fehler. Und wenn man solche Änderungen, 100 Dateien, in denen jeweils eine Zeile geändert wurde, im selben Merge-Request behandelt, wie tatsächlich dann das eigentliche Feature, was entwickelt werden sollte, dann macht es das für alle total unübersichtlich, weil man nie sehen kann, war das jetzt eine Änderung, die durch so ein automatisiertes Renaming erfolgt ist oder ist da wirklich, wie soll ich sagen, Schweiß und Gedanken von dem entwickelnden Menschen reingeflossen und da müsste man drüber nachdenken.

  3. 12:22

    Deswegen machen wir es da gerne auch so, bei solchen automatisierten Refactorings, die eigentlich automatisch passieren und wo man nicht jede Datei nachschauen muss, weil das, wie gesagt, von einer IDE gemacht wurde, eigenen Merge-Request und dann hat man einen separaten zweiten Merge-Request, der sich nur mit den tatsächlich relevanten Änderungen beschäftigt. Eine Daumenregel können wir da aber trotzdem natürlich rausnehmen, dass wir wirklich versuchen, kleine Merge-Requests zu machen. Also, dass man nur wenige Änderungen da drin hat, dass man definitiv gescoped hat auf einen bestimmten Task.

  4. 12:53

    Einfach, damit der PR schnell durchgeht, damit er einfach zu lesen ist, damit man nicht so ein riesen Gedankenkonstrukt aufbauen muss. Also, kleine PRs sind einer der wichtigsten Punkte für gute PRs, meiner Meinung nach. Genau, und dafür ist es auch unumgänglich, dass die Person, die so ein Merge-Request gestellt hat, sich selber in GitLab nochmal diesen Merge-Request anschaut. Also, wirklich jede einzelne Datei nochmal durchgehen und überlegen, hat das wirklich mit diesen Dingen zu tun? Weil häufig genug, im Frontend ist es üblicherweise so, dass so eine Package-Log aktualisiert wurde. Und dann gerät die damit rein.

  5. 13:28

    Obwohl eigentlich an den Paketen in diesem Merge-Request gar nichts geändert wurde. Oder man hat eine Debugging-Ausgabe noch drin gehabt und vergessen, die rauszunehmen oder sowas. Also, da schadet es nicht. Und da halte ich die Leute auch immerzu an, selber noch einmal durchklicken und schauen, gehört das hier auch alles rein, was ich hier gemacht habe. Und das kann man in GitLab ganz wunderbar außerhalb vom Code-Editor nochmal mit einem frischen Blick sich anschauen. Ich habe bei mir, wenn ich programmiere, immer so zwei doppelte Böden. Und das Erste ist, dass ich Tests schreibe und dabei natürlich über meinen eigenen Code nochmal nachdenke

  6. 14:03

    und den vielleicht auch schon ein Stück weit revieue. Und das Zweite ist eben, wenn ich ein PR gestellt habe, dann mache ich mir bei uns jetzt GitLab auf und scroll wirklich die Datei nochmal durch. Und es gibt eigentlich keinen Fall, in dem man dann nicht doch noch einen Konsole-Log oder irgendwas findet, wo man nicht noch was nachbessern kann und ihn dann wirklich final zum Review stellen kann. Jetzt haben wir gerade gesprochen, es soll nur das rein, was wirklich mit der Aufgabe zu tun hat. Gleichzeitig ist es ja aber so, dass man immer mal wieder, wenn man auch insbesondere in einer alten Code-Base unterwegs ist,

  7. 14:34

    dass man da mal Stellen sieht, wo man denkt, oh, das ist aber nicht ganz so schön gelöst, da müsste man nochmal rein. Und hier diese Stelle, oh, die können wir richtig schön refactoren, die entspricht nicht mehr unseren Standards und sowas. Und das ist so eine fortlaufende Debatte bei uns zwischen... Ja, sehr kontrovers. Eigentlich möchten wir nur die Sachen committen und den Merge-Request stellen, die unmittelbar mit der Aufgabe zu tun haben. Aber gleichzeitig möchten wir auch fortlaufend unsere Code-Base aktuell halten. Das heißt, auch Stellen im Code, die nichts unmittelbar mit der Aufgabe zu tun haben,

  8. 15:08

    wollen wir anfassen, um die zu modernisieren und die Sachen frisch zu halten, um vielleicht mit das UI ein bisschen anzupassen und so kleine Bugs auszubessern, für die keiner eine einzelne Aufgabe gestellt hat. Idealerweise wird man natürlich total viele unterschiedliche Merge-Requests stellen. Aber das ist natürlich auch ein bisschen aufwendig. Und deswegen haben wir so eine gewisse, wie soll ich sagen, verzeihen wir den Leuten, will ich es mal nennen, oder ermuntern sie auch dazu, kleinere Änderungen, wenn sie irgendwelche Verbesserungen finden, mit reinzunehmen, die ihnen so bei der täglichen Arbeit begegnet sind.

  9. 15:43

    Einfach um sicherzustellen, dass der Code aktuell bleibt. Da kann man dann übrigens wieder gut das, was ich vorhin gesagt habe, verwenden. Und einen kurzen Kommentar reinschreiben, wo man dann schreibt, hey, ich weiß, das hat nichts mit der Aufgabe zu tun, aber das ist mir aufgefallen, lass uns das mal noch mit reinnehmen oder so. Ja, das war ein langer Weg, wie wir da hingekommen sind, weil früher waren wir da sehr strikt, oder vor allem du, Kay, und du hast gesagt, es kommt wirklich nichts rein, was nichts mit der Aufgabe zu tun hat. Und wenn man was refactoren will, was ändern will, dann macht man dafür einen neuen PR.

  10. 16:09

    Und das hat dazu geführt, dass es eigentlich keiner so richtig gemacht hat. Oder aber, dass ganz viele Tasks erstellt wurden, muss ich später noch machen. Und dann wurde es aber nie gemacht. Und dabei sind es eigentlich, wenn man da schon drüber stolpert in der Code-Zeile, sich vielleicht damit beschäftigt hat, sind es da nur wenige Minuten, die man brauchen würde. Und so ist sicherlich der Modus besser, auch wenn die PRs dann manchmal so ein bisschen Fremdcode enthalten. Aber da ist auch so ein bisschen der gesunde Menschenverstand von jedem einzelnen Entwickler, von jeder Entwicklerin gefragt,

  11. 16:38

    dass man wirklich abschätzt, das ist jetzt so klein und passt noch so gut in diesen PR rein, dass sie das jetzt mit reingeben. Es ist ganz wichtig für mich und auch für dich und auch für unsere Kolleginnen und Kollegen, dass diese Merge-Requests ein Tool sind, das uns insgesamt hilft. Und das ist, glaube ich, bei vielen Tools so in der Entwicklung, die man so hat, die müssen einem helfen. Und wenn man irgendwie merkt, dass sie einem Steine in den Weg legen und gar nicht produktiv sind, dann stimmt da irgendwas nicht. Und so ist es auch mit diesen Merge-Requests. Wir haben da zwar jetzt so eine kleine Guideline,

  12. 17:10

    die wir heute vorstellen, aber das ist ein flexibles System, also das entwickelt sich weiter. Ich muss eine Sache einschieben, kurz zu diesen kleinen Fixes. Ach so, ja. Weil mir das ganz wichtig ist. Wir haben bei uns festgelegt, dass wir dann immer definitiv einen eigenen Commit zumindest machen für die einzelnen Fixes, für die Refactorings, sodass man mindestens in der Code-Historie im Log dann hinterher sieht, da ist jetzt noch was Separates gemacht worden und wenn ich das wieder zurückrollen würde, dann könnte ich das auch machen. Also ein eigener Commit ist Pflicht, aber kann dann trotzdem

  13. 17:40

    in dem Merge-Request insgesamt enthalten sein. Genau. Ich bin sicher, wir sprechen nochmal darüber, wie wir tatsächlich dann die ganze Arbeit gestalten bis zu diesem Merge-Request. Das ist ja so ein bisschen das Ende der Fahnenstange, so eine Aufgabe, wenn man möchte. Aber genau, du hast ganz recht. Also kurzum, nur Sachen rein, die rein sollen. Wenn man irgendwelche Refactorings reinbringt, dann gerne kommentieren oder sowas, damit das kenntlich ist. Und dann kommt eigentlich der wichtigste Schritt, würde ich sagen, der bei so einem Merge-Request stattfindet, bevor überhaupt irgendjemand anders draufgeschaut hat,

Schritt 2: Pipelines 18:03–23:33

  1. 18:11

    nämlich unsere automatischen Pipelines. Und wir haben am Anfang, also in der allerersten Folge, als wir über Qualität gesprochen haben, über so Coding-Qualität gesprochen, wie Linting, automatisierte Tests und solche Dinge. Und das sind alles Aufgaben, die wir, soweit es eben geht, automatisiert in so einer Pipeline abfrühstücken. Das bedeutet, wenn der Merge-Request gestellt wurde, dann läuft ein automatisches Skript durch, das ganz verschiedene Dinge automatisiert auf dem Code testet. Das sind zum Beispiel Unit-Tests, habe ich schon gesagt. Also die Aufgabe muss natürlich neue Unit-Tests enthalten,

  2. 18:50

    plus die ganzen bestehenden Tests, die in der Code-Base schon drin sind, müssen alle durchlaufen, bevor der Merge-Request irgendwie behandelt wird. Das zweite sind so Linting-Regeln. Also wir haben gewisse, wie soll ich sagen, sprachliche Eigenschaften definiert, beziehungsweise Sachen festgelegt, wie zum Beispiel in JavaScript wollen wir immer ein Semikolon am Ende einer Zeile haben. Das geht mit und das geht ohne. Wir haben entschieden, das wollen wir mit, konsequenterweise. Deswegen haben wir eine Linting-Regel, die das immer nachstellt. Was ich nicht möchte, ist, dass der händische Code-Review,

  3. 19:25

    der ja super aufwendig ist, kommen wir nachher noch drauf, sich mit solchen Dingen beschäftigen muss. Sondern alles, was automatisiert erfasst und bemerkt werden kann, soll automatisiert auch abgehandelt werden, idealerweise schon in der IDE des Programmierers, das beim Speichern sowas gemacht wird, wie ein Semikolon ergänzen und wir uns dann im Review, im händischen Review tatsächlich nur mit Sachen beschäftigen müssen, die software-systemische Implikationen haben, die Auswirkungen auf andere Bereiche der Anwendungen haben, wo wir Usability und User Experience untersuchen und sowas. Und alles,

  4. 20:02

    was man sonst automatisiert machen kann, machen wir automatisiert. Ich habe das in früheren Aufträgen festgestellt, dass der Fokus ein ganz anderer ist, wenn wir zum Beispiel dieses Linting nicht machen, also wirklich so Code-Style-Geschichten, dann scrollt man eigentlich den PR durch und sucht eben nach so Styling-Sachen und freut sich immer, wenn man eine Zeile findet, wo eine Klammer nicht eingerückt ist oder sowas. Oder man findet Schreibfehler, aber man konzentriert sich nicht auf die Architektur von der ganzen Sache und das ist natürlich ganz problematisch und deswegen wollen wir das möglichst wegautomatisieren,

  5. 20:35

    genauso wie du es erklärt hast. Genau, wir haben dann zum Beispiel erst Lint konkret für unsere Frontend-Sachen, wo wir ein eigenes Regelsatz hinterlegt haben und das wird zum einen bei der Entwicklung direkt verwendet bei sowas wie Actions on Save, also wenn jemand in seinem Editor was speichert, wird automatisch diese Linting-Regel gecheckt und aber auch angepasst, also es muss sich niemand darüber kümmern, dass man wirklich die richtigen Einrückungen macht, dass man lernt, wo welche Klammer hin muss oder sowas, sondern er schreibt einfach runter, was er denkt, drückt auf Speichern und der Linter

  6. 21:06

    macht das für ihn. Das will ich auch nicht, dass irgendein Kollege sich damit beschäftigen muss, wo macht er jetzt die öffnende, geschweifte Klammer bei so einem If-Statement hin oder sowas. Ja, oder das alphabetische Sortieren der Imports. Ja, genau solche Sachen. Also vielleicht einmal kurz aufgezählt, was wir so machen. Unit-Tests, habe ich schon gesagt, generell automatisierte Tests in allen Belangen. Linting, Stichwort ES-Lint. Was wir aber auch machen, sind solche Sachen wie mal ein Test-Bild bauen zum Beispiel, so über TypeScript zum Beispiel, mal so ein Frontend-Bundle zu machen, auch wenn das nachher

  7. 21:38

    gar nicht produktiv eingesetzt wird, sondern einfach nur zu schauen, ob da irgendwelche Compile-Errors stattfinden, ob TypeScript da irgendwelche TypeScript-Probleme hat oder sowas. Da machen wir auch noch bei anderen Sachen irgendwelche Reportings. Wir haben zum Beispiel SonaCube in manchen Projekten, die auch nochmal so automatisiert irgendwelche Regeln checken und solche Dinge. Also kurzum, alle solche Quality-Sachen, die man automatisiert erfassen kann, werden in der Pipeline automatisiert abgespielt und der Merge-Request ist erst dann ready to review, sag ich mal, wenn die Pipeline grün ist,

  8. 22:15

    klassischerweise. Ich habe jetzt mir im Vorhinein ein Video von GitLab angeguckt, weil ich mal wissen wollte, wie die da so als Referenz machen mit dem Merge-Request und die haben in ihrer Pipeline einen Step drin, der Review heißt und das heißt, das Review ist Teil der Pipeline des Merges in den Main-Branch und erst wenn der Review abgeschlossen ist, drückt der Reviewer dann auf also auf den nächsten Schritt, also auf weiter in der Pipeline und dann läuft es weiter. Fand ich erstmal einen interessanten Gedankenansatz. Ich hoffe, ich habe das auch richtig verstanden, so wie wir das machen. Können wir einfach mal

  9. 22:48

    so als Gedankenspiel bei uns mit aufnehmen. Ich bin noch nicht so ganz sicher, ob ich davon begeistert bin. Ja, wir haben natürlich auch ein relativ kleines Team, wo man schnell unter der Hand mal mit Kollegen schreiben kann und sagen, hey, guck mal hier, ist das und das und sowas. Das spielt uns natürlich ein bisschen da rein. Also die Merge-Requests sind so streng, wie sie sein müssen, aber natürlich haben wir immer die zwischenmenschliche Kommunikation auch und ich glaube, bei solchen Unternehmen wie bei GitLab oder generell bei öffentlichen Repositories, bei öffentlichen Repositories, so wollte ich sagen,

  10. 23:20

    wo jeder einen Merge-Request hinstellen kann, die haben natürlich sehr viel konkretere Anforderungen, auch wie so eine Beschreibung aussehen muss und solche Sachen, das haben wir ja alles nicht. Kann natürlich trotzdem hilfreich sein, sich da mal anzugucken, wie die das machen. Dann ist der Merge-Request fertig im Sinne von ready to review, also der Kollege oder die Kollegin findet das fertig, findet das gut. Was wir dann haben, ist einen Review-Pool, mit dem ich noch nicht so ganz zufrieden bin. Aber was er dann macht, im Moment ist das so ein Slack-Channel und da schreibt er rein, ey, schau mal hier,

Schritt 3: Review Pool mit KollegInnen 23:33–26:51

  1. 23:54

    hier ist ein Link zu diesem Merge-Request, da geht es um dies und das, schaut euch das mal bitte an. Wir haben bei uns im Unternehmen so eine, wie soll ich sagen, Merge-Request-Hierarchie, möchte ich mal sagen, nämlich einen großen Pool von gleichberechtigten Entwicklerinnen und Entwicklern, die gegenseitig sich den Code anschauen können und dann stecken Felix und ich als Hüter der Qualität, möchte ich mal sagen, oben drüber. Und die Idee ist, dass wenn so ein Merge-Request fertig ist, dann wird der erst einmal in den Pool reingeworfen, dann werden Kolleginnen und Kollegen animiert, sich das anzuschauen

  2. 24:29

    und schon zu kommentieren und erst dann wird der an uns weitergeleitet. Das haben wir vor gar nicht so langer Zeit mal eingeführt aus mehreren Erwägungen. Zum einen einfach, um die Kolleginnen und Kollegen zu Schulen selber so Merge-Requests zu machen, weil ich finde, wenn man sich selber mit so einem Code von anderen Leuten beschäftigt, dann lernt man selber auch unglaublich viel und fängt an, auf Dinge zu achten, die einem vielleicht vorher beim eigenen Schreiben nicht so aufgefallen sind. Man lernt, sich in Sachen so schnell reinzudenken und solche Dinge. Und das ist einfach ja auch eine Wissensvermittlung,

  3. 25:06

    die wir im Team haben wollen. Dass Leute auch mal Code sehen, den sie nicht direkt geschrieben haben, aber ungefähr wissen, was in der Anwendung sonst so passiert. Ich habe ganz oft Code-Teile, die ich mir so ein bisschen isoliert rauspicke und wo ich dann denke, boah, das kann doch überhaupt nicht funktionieren und dann probieren wir das aus und dann läuft das doch. Ganz spannend, was man da, also wie andere Entwickler dann auch entwickeln. Und das muss ja gar nicht unbedingt ein Review sein, um zu schauen, was hat der andere falsch gemacht, in Anführungszeichen, sondern das kann ja auch, sein,

  4. 25:34

    dass vielleicht nicht so erfahrene Kollegen sich den Code von anderen anschauen und sich denken, guck mal, so hat der das gelöst, das wäre mir gar nicht eingefallen. Das ist ja eine interessante Strategie, das so und so zu machen. Vielleicht übernehme ich das beim nächsten Mal auch. Also die Idee ist, im Team untereinander zum einen ein bisschen zu schulen, auch Reviews zu machen, aber auch ein bisschen zu lernen, was geht im Projekt sonst so. Wenn wir fünf Kolleginnen haben, die im selben Projekt sitzen, dann schadet es sicherlich nicht, wenn die auch mal andere Teile im Rahmen von so einem Review sehen.

  5. 26:08

    Das eigentliche Format, da bin ich bei uns noch nicht so ganz glücklich, weil das eben über so einen Slack-Channel läuft, aber so vom Konzept her finde ich das super stark. Und wenn dann die das durchgeschaut haben, dann approven die das und so kann man dann immer gut nachhalten, ah, schau mal hier, der Kollege oder die Kollegin hat sich das schon angeschaut. Früher bei Bitbucket konnte man immer mehrere Assignees zuweisen, also mehrere Reviewer quasi und das war ganz praktisch, weil man dann einfach alle, die in Frage kamen, da zugewiesen hat und die Leute, die gerade Zeit hatten, haben sich das Ding dann geschnappt.

  6. 26:42

    Bei GitLab geht das nur in der bezahlten Variante, deswegen nutzen wir das Feature im Moment nicht, weil wir sonst keine bezahlten Features brauchen im Moment. Das sind wir auf der kostenlosen Variante. Dann gehen wir mal davon aus, im zweiten und dritten Schritt ist jetzt der Merge Request soweit fertig. Soll heißen, der Mensch, der den Merge Request gestellt hat, findet den in Ordnung und Leute aus dem Pool finden den auch in Ordnung. Dann geht es weiter zum finalen Merge Request, möchte ich es mal nennen. Das heißt, dann wird dann wirklich der Merge Request explizit an mich zum Beispiel zugewiesen

Schritt 4: Finales Review 26:51–40:36

  1. 27:17

    oder an Felix für das finale Review. Und das hat sich auch ein bisschen gewandelt im Laufe der Zeit und unser aktueller Stand ist, dass wenn der Merge Request eine gewisse Größe oder eine gewisse Komplexität erreicht hat, dass wir das im One-on-One Live Review machen. Soll heißen, ich rufe den Kollegen und die Kollegin an, teile meinen Bildschirm und dann gehen wir gemeinsam bei mir in GitLab den Merge Request durch. Und das hat sich als super wertvoll erwiesen aus verschiedenen Gründen. Und das möchte ich gerne ein bisschen ausführen, weil das, glaube ich, ein sehr wichtiger Punkt ist, von dem ich glaube,

  2. 27:55

    dass der uns ein bisschen besonders macht. Das eine ist ganz trivial zwischenmenschlicher Kontakt. Wir hatten vorher schon mal eine Folge über Homeoffice und die Vor- und Nachteile. Und bei allen Vorteilen ist ein Nachteil einfach, dass man die Kollegen selten sieht. Und so ein One-on-One Live Review, wo wir zusammen im Slack sitzen und reden, ist zumindest eine Möglichkeit, um einfach in Kontakt zu bleiben. Da geht man nicht direkt in medias res, wenn man die anruft, sondern plaudert vielleicht noch mal kurz, sagt, hey, wie geht's? Hier redet über irgendwelche Alltagsdinge. Und das ist eine super Gelegenheit

  3. 28:33

    für mich, einfach mit den Kollegen im Gespräch und im Kontakt zu sein. Und das hilft auch dabei, wirklich so eine positive Atmosphäre im Review zu haben. Ich habe das in anderen Firmen schon erlebt, dass ein Review, ja, dass man da fast schon ein bisschen Angst hatte, dass dann da Punkte aufkommen. Und bei uns ist es eben hoffentlich anders. So, dass wir wirklich den Code in den Vordergrund stellen und nicht Entwickler und nicht persönlich werden, sondern wirklich sehr positiv schreiben und Verbesserungsvorschläge einbringen und die im besten Fall eben auch dankbar angenommen werden. Ich kann das zumindest von mir sagen,

  4. 29:06

    wenn ich was in die Review stelle und dann denke ich natürlich immer, ich habe den besten Code der Welt geschrieben und dann kommt doch jemand und hat noch einen Verbesserungsvorschlag und dann denke ich so, yo, das ist es, das ist cool, da werde ich noch besser durch. Und das ist genau dieser Spirit, den wir haben wollen. Also wir wollen kein Fingerpointing machen oder so, sondern wir wollen wirklich den Leuten helfen, besser zu werten, gerade auch den nicht so erfahrenen Entwicklern und Entwicklerinnen zu helfen. Und man muss das insgesamt wirklich positiv sehen. Man muss coole Codestellen auch hervorheben.

  5. 29:33

    Dann kann man den Comment auch direkt resolven, also dass da gar nicht weiter drüber gesprochen werden muss, aber einfach mal zu sagen, boah, hier, das ist richtig cool gelöst, das finde ich ganz wichtig im PR. Das ist ganz, das ist ganz, ganz wichtig, was du sagst. Und das war bei uns auch ein langer Prozess, bis wir das hatten. Weil wenn man sich nur diese Merch-Requests in GitLab zum Beispiel anschaut, dann tendiert man, oder zumindest habe ich das lange Zeit gemacht, dazu eben nur kritische Dinge hervorzuheben. Und selten tatsächlich einen Kommentar an einer Stelle zu schreiben, wo was Positives passiert ist,

  6. 30:05

    einfach weil es dann eine Stelle im Code ist, die erstmal Aufmerksamkeit bringt. Aber in so einem One-on-One-Review, wenn man eh den Bildschirm teilt, da so Stellen hervorzuheben und zu sagen, hey, guck mal hier, das ist super gut gewesen, voll die gute Idee, daran hätte selbst ich nicht gedacht oder sowas. Und ehrlich im Feedback dann auch zu loben, das ist, glaube ich, ein ganz, ganz wichtiger Baustein, damit diese Merch-Requests insgesamt nicht so ein Gefühl haben von, ich habe eine Arbeit geschrieben und jetzt wird sie korrigiert und dann kommt sie mit dem Rotstift zurück, sondern wir schaffen

  7. 30:38

    gemeinsam als Team etwas und alle helfen so gut es geht mit. Ja, genau. Und wenn ich das dann live mit dem Kollegen und der Kollegin anschaue, dann habe ich auch gemerkt, dass ich viel mehr involviert bin. Einerseits, weil ich natürlich gleichzeitig nicht abgelenkt bin, weil ich habe den ja an der Strippe und muss mich dann damit beschäftigen. Aber andererseits, weil auch die Hemmschwelle viel niedriger ist, mich in den Code einzuarbeiten, weil ich habe die Person, die das gebaut hat, hier neben mir und wenn dann irgendeine Stelle nicht so ganz klar ist, dann kann ich eben schnell fragen, hey, schau mal,

  8. 31:10

    wie sind denn hier nochmal die Zusammenhänge? Das weiß ich gerade nicht mehr im Kopf und sowas. Und damit beschäftige ich mich auch viel, viel intensiver mit dem Code und bin gezwungen, gewissermaßen, da auch Zeit reinzustecken und mich wirklich damit zu beschäftigen, statt einfach nur zu überfliegen und zu hoffen, dass es klappt. Das ist wirklich ein ganz wichtiger Punkt. Ganz viele sagen ja, ein Code Review ist natürlich extrem aufwendig und es ist ja vor allem deshalb aufwendig, weil man Stellen hat, in die man sich wirklich reindenken, in die man sich einlesen muss und wenn man jemanden direkt dabei hat,

  9. 31:39

    dann kann man dem sagen, hier, erklär mir doch mal eben diese Codestelle und ich habe jetzt vielleicht gerade nicht mehr so ganz präsent, wie in JavaScript eine Reduce-Funktion funktioniert oder im PHP und das kann mir jemand dann super schnell kurz erklären, dann weiß ich Bescheid, da muss ich nicht googeln und das macht so ein PR dann eigentlich deutlich weniger aufwendig auch am Ende. Ja, wobei, man muss schon sagen, unsere PRs sind super aufwendig und das hat alleine damit zu tun, dass wir die nicht nur sehen als Code Review, also im Sinne von, was ist denn bei der konkreten Zeile, die hier geschrieben wurde,

  10. 32:16

    gut oder schlecht oder was kann man besser machen, was sind andere Konzepte, was hat das für Abhängigkeiten, oder sowas, sondern wir benutzen diese Code Reviews auch ganz bewusst und sehr ausführlich für so interne Weiterbildungen. Weil häufig genug, wenn da irgendetwas ist, was jetzt der Linter nicht gesehen hat, dann hängt das meistens damit zusammen, dass da irgendwelche übergreifenden Konzepte hinter stecken, die vielleicht jetzt der Person, die das geschrieben hat, nicht so ganz klar sind und das kann man dann viel besser als alles andere zum Anlass nehmen, um über diese übergreifenden Konzepte

  11. 32:50

    zu sprechen. Wenn ich dann zum Beispiel sehe, oh, hier ist die Benennung von einer Variable vielleicht nicht ganz eindeutig, dann könnte ich entweder reinschreiben in so einem schriftlichen Code Review, benenn das mal um in so und so, aber wichtig ist ja für mich, dass die Person meine Gedankengänge nachvollziehen kann, warum ich finde, dass das eine ungünstige Benennung ist und welche Strategien ich anwenden könnte, um eine bessere Benennung zu finden, damit ich idealerweise jetzt bei diesem Merge Request vielleicht von mir aus auch eine Stunde darüber rede, warum diese Benennung unglücklich ist,

  12. 33:25

    aber dafür weise ich danach, die Person, mit der ich das besprochen habe, die hat jetzt ein tiefgreifendes Verständnis davon, nicht nur warum das eine ungünstige Benennung ist, sondern wie man auch eine bessere finden kann. Wir haben darüber gesprochen, Stichwort Entities und solche Sachen und dieser Podcast hier stammt ja auch aus den Merge Requests, nämlich, dass ich eine Stunde über Sachen rede, die mir im Code auffallen, weil die wichtig sind und jetzt rede ich halt eine Stunde hier vor dem Mikrofon statt mit Leuten alleine. Genau. Einen wichtigen Punkt finde ich noch, dass man als derjenige,

  13. 34:00

    der den PR gestellt hat, nicht alles einfach blind übernimmt, was mir ein Reviewer vorschlägt, sondern dass ich wirklich in eine Diskussion reingehe und wir haben sehr, sehr oft den Fall, dass Sachen angemerkt werden, die hinterher tatsächlich nicht übernommen werden, weil derjenige, der es programmiert hat, sich dabei was gedacht hat, weil das sinnvoll ist und ja, weil in der Argumentation dann hinterher rauskommt, ah, ist vielleicht auch ganz gut so, wie es umgesetzt ist. Das ist ein ganz wichtiger Punkt, den gerade junge Leute dann erstmal lernen müssen, dass sie nicht von dem Entwickler, der das dann

  14. 34:31

    reviewt, alles blind einfach übernehmen, sondern wirklich auch mal, ich sag mal in Anführungsstrichen, Widerworte geben und sagen, nee, nee, das habe ich mir schon so gedacht und das ist schon so richtig, wie ich es gemacht habe. Total, das ist der schönste Moment für mich als Lehrer, sag ich es mal, wenn ich eine Code stelle, anmerke und sage, hey, hier, schau mal, hier musst du das so und so machen, also müssen im Sinne von, ich rege Verbesserungen an, das ist niemals bei uns so, du musst das jetzt so machen und der Kollege sagt aber, nee, habe ich daran gedacht, aber hier und da sind diese Abhängigkeiten

  15. 35:01

    und deswegen geht das hier nicht, das ist der schönste Moment und ich finde auch daran erkenne ich, wie sich die Kolleginnen und Kollegen weiterentwickeln, am Anfang ist es nämlich so eine klassische von oben nach unten Situation und ich sage, hey, schau mal, diese Variable heißt blöd, weil diese und jene Gründe und dann wird das einfach kommentarlos geändert, aber mit den Jahren wird das dann immer mehr gechallenged und dann heißt es dann, ja, aber guck mal, hier sind noch diese Sachen und hier sind diese Sachen und diese Sachen und je weiter die Kompetenz fortschreitet von den Kollegen, desto mehr

  16. 35:36

    ist das so eine Sache von, ich will jetzt gar nicht über diese konkrete Codestelle sprechen, sondern ich schlage nur allgemeine Konzepte vor, die man hier vielleicht anbringen könnte und du als Entwicklerin oder Entwickler entscheidest, ob das hier sinnvoll ist, weil natürlich, ich habe jetzt gerade den Code zum ersten Mal gesehen vielleicht, die anderen haben sich da tagelang mit beschäftigt, die sind viel, viel tiefer drin und wenn die dann sagen, ja Kay, ich weiß diese Konzepte und ich habe mich aus diesen und jenen super guten, nachvollziehbaren Gründen dafür entschieden, dann ist auch einfach

  17. 36:09

    irgendwann eine Stelle erreicht, wo ich nicht mehr sagen kann, ja, aber es ist blöd, mach das mal anders, sondern der Mensch hat sich damit beschäftigt und wenn ich weiß, dass der die Konzepte, die mir wichtig sind, berücksichtigt, dann muss ich das auch hinnehmen. eine Schleife möchte ich ganz gerne noch drehen und zwar habe ich im Vorhinein gelesen, dass man bei PRs keine Aufmerksamkeit für die kleinsten Details haben soll und nicht so Nitpicking machen soll, also so kleinere Sachen auch mal durchgehen lassen soll. Habe ich jetzt ein bisschen vereinfacht ausgedrückt, aber letztlich der Sinn dahinter war ungefähr so

  18. 36:40

    und das habe ich in mehreren Artikeln gefunden und das, finde ich, spiegelt nicht das wieder, was wir machen. Wir wollen nämlich genau jedes Mini-Detail im PR berücksichtigen und das führt natürlich dazu, dass die ersten PRs von Leuten, die neu im Team sind, sehr, sehr aufwendig und lang werden, aber 90 Prozent der Sachen stellen sich nach zwei PRs ab und wenn man die jedes Mal akzeptieren würde, diese kleinen Details, dann würden wir immer weiter eine Verwässerung von unserer Code-Qualität kriegen, von unserem Code und gerade die Details sind es, finde ich, was es auch ausmacht. Also das finde ich,

  19. 37:17

    dass wir das nicht so stehen lassen können und dass es bei uns schon wichtig ist, auch im Detail zu gucken und wenn wir in den PR durchwinken, dann ist der nach unserem Dafürhalten auch in Anführungsstrichen perfekt und das ist so unser Anspruch dabei. Alleine schon aus dem Grund, weil ja häufig genug, wenn man selber was entwickelt, man sich auch schon an der bestehenden Code-Base orientiert. Wie wurde das denn damals gemacht? Ich habe hier eine sehr ähnliche Funktion, dann schaue ich mir das mal an und orientiere mich da dran und wenn dann damals schon so Unsauberkeiten sich eingeschlichen haben,

  20. 37:49

    dann kann ich mich jetzt natürlich daran orientieren und denke, ja, es ist im Mainbranch, es wird ja wohl richtig sein und so setzen sich solche Sachen dann fort. Also ich habe häufig genug in Merge-Request-Situationen, wo ich auch denke oder wo ich anmerke, dass hier was nicht optimal ist und dann kommt, ja, aber an der Stelle ist das schon so und so und das wird schon häufig noch angenommen. Aber auch innerhalb eines Projektes entwickelt sich natürlich so ein Qualitätsstandard und dadurch ändern sich solche Dinge. Also ich bin voll deiner Meinung, der Merge-Request sollte so perfekt wie möglich sein,

  21. 38:21

    weil wir unseren Mainbranch als perfekt ansehen und auch als Orientierung für andere Leute. Genau, das heißt natürlich längst nicht, dass bei uns perfekter Code rauskommt, also um Gottes Willen. Aber wenn wir zumindest alles dafür tun, dass er möglichst nah rankommt, dann dürfte die Qualität ganz in Ordnung sein. Eine Sache noch, die gerade diese Detailsachen etwas einfacher macht, finde ich gerade bei GitLab ganz praktisch, sind diese Code-Vorschläge, die man direkt im PR einbringen kann. Also wenn ich eine Zeile habe, habe ich so einen Button mit so einem Code-Symbol und dann kann ich direkt

  22. 38:53

    einen Code-Vorschlag einbringen, also kann wirklich so inline eine Code-Alternative dann anbieten und derjenige, der den PR gestellt hat, kann das mit einem Klick übernehmen, also kann das einfach submitten. Und das ist ziemlich cool, wenn es zum Beispiel, wenn ich jetzt irgendwas groß oder klein schreiben würde, so Mini-Geschichten da drin sind, dann kann ich das eben als Code-Vorschlag machen, der andere kann es einfach per Klick übernehmen und dann ist das ein Mini-Detail, was aber auch in einer Sekunde abgehandelt ist. Was vielleicht noch so eine kleine Besonderheit ist, die daraus resultiert,

  23. 39:25

    eben weil wir diese Reviews machen im One-on-One mit den Leuten direkt am Telefon, sag ich mal. Dadurch werden die Kommentare, die wir dann im Code hinterlegen, nicht so sehr ausdrucksstark. Das bedeutet, ich habe dann zwar eine Stunde über eine Stelle im Code gesprochen, wo eine Benennung vielleicht nicht ganz sauber ist, aber was ich dann tatsächlich im Code-Review reinschreibe, ist umbenennen, weil er und ich ja drüber gesprochen haben. Und das ist natürlich ein bisschen schwierig, das weiß ich, insbesondere wenn dann dieser Merge-Request aus irgendeinem Grund länger liegen bleibt oder der Kollege krank wird

  24. 40:03

    und sich jemand anders damit beschäftigt, dann ist diese Stelle im Review natürlich nicht so richtig aussagekräftig, weil dann da nur steht umbenennen und keiner weiß mehr, worum es geht. Man muss dann also das Hintergrundwissen haben aus diesem einstündigen Gespräch, um noch zu wissen, worum das hier geht. Aber ehrlicherweise kommt das bei uns selten genug vor, dass es sich auf der anderen Seite nicht lohnen würde, nachdem man eine Stunde drüber gesprochen hat, mündlich, nochmal schriftlich da ein halbes Pamphlet aufzusetzen, warum das jetzt eine gute Änderung ist. Aber das ist ein Risiko. Ja, dann haben wir jetzt,

Schritt 5: Revisionen 40:36–42:56

  1. 40:37

    glaube ich, den, sag ich mal, den One-on-One-Review abgehakt, entweder mit dem Outcome, dass alles perfekt ist und es direkt gemerged wird, übrigens dann mit Squash-Commits und Feature-Branch-Deleten. Üblicherweise sammeln sich dann aber über so ein Review einige Kommentare an und die werden dann an den Entwickler zurückgespielt. Wir machen das bei uns in GitLab ganz einfach so, dass wir über den Assignee kennzeichnen, wer jetzt gerade da an dem Branch eine Aufgabe hat. Das heißt, es wechselt dann. Am Anfang, wenn jemand den Merge-Request erstellt, dann wird erst mal sich selber assigned, um zu zeigen,

  2. 41:21

    ich bin da jetzt gerade dran und wenn es dann aber zum Review an zum Beispiel mich übergeht, dann werde auch konkret ich assigned und der Ersteller unassigned und so wechselt das immer anhand dieses Assignees hin und her, wer jetzt gerade was machen muss. Wenn ich das gereviewt habe und dann noch was geändert werden muss, assign ich das wieder zurück und das ist dann das Signal für den Kollegen oder die Kollegin, da jetzt handgreiflich zu werden. Und nachdem er alles geändert hat oder vielleicht kommentiert, warum das eine schlechte Idee ist, machen wir es bei uns so, dass wir mit so kleinen Daumen-Emojis

  3. 41:57

    anzeigen, okay, ich habe deinen Kommentar gelesen und umgesetzt und dass nur derjenige einen Thread resolven darf, der ihn auch erstellt hat. Also die Idee ist, jemand merkt eine Zeile im Code an, es wird geändert, es wird durch ein Emoji angezeigt, dass man diesen Kommentar gesehen und umgesetzt hat und dann wird der Merge-Request wieder zurück an denjenigen geschickt, der diesen Thread erstellt hat, um entweder das zu resolven, wenn es im Sinne der Umsetzung ist oder eben nochmal einen weiteren Kommentar zu machen. Und so spielen wir uns den Ball dann hin und her und die Kommentare werden mal mehr

  4. 42:36

    und mal weniger und am Ende wird es dann gemerged. Das kann tatsächlich unterschiedlich lange sein, je nach Aufwand, aber so kann es tatsächlich schon mal vorkommen, dass wir an einem Merge-Review ein paar Stunden hängen, wenn da irgendwelche tiefgreifenden Sachen aufgefallen sind. Wir sind eben der Meinung, dass sich das für uns lohnt, weil wir im Nachhinein viel, viel weniger mit Bugs und so weiter zu tun haben und gerade dieser Lerneffekt, über den du jetzt schon viel erzählt hast, der ist natürlich immens, dass wenn ich wirklich mal durch fünf, sechs längere PRs durchgegangen bin, dann weiß ich einfach

Vorteile 42:56–43:37

  1. 43:13

    hinterher, wie der Hase läuft, wie gecodet wird, wie das Team arbeitet, wie die anderen Leute denken. Das ist so wertvoll und wir merken das wirklich, dass der erste PR, die ersten PRs von den Leuten, die dauern eben zum Teil dann mehrere Stunden, also jetzt nicht ein Review, sondern insgesamt zusammengenommen und hinterher gehen PRs oft in zehn Minuten durch und es ist wirklich nicht mehr viel Aufwand und trotzdem geniale Qualität. genau, aber da habe ich schon ein kleines Downside angesprochen, vielleicht gehen wir da jetzt kurz über, was denn vielleicht nicht optimal ist an diesem Ablauf, nämlich sehr

Nachteil 1: Zeitaufwändig 43:37–45:44

  1. 43:47

    zeitintensiv, das kann man so und so sehen, aber ich glaube, das ist schon für uns eine enorme Luxusposition, dass wir uns die Zeit nehmen können und auch die Zeit nehmen, eben solche aufwendigen Reviews zu machen, weil das ist einfach sehr, sehr zeitintensiv und dauert schon mitunter dafür, dass sich jemand vielleicht doppelt so lange mit dem Merge Request beschäftigt, als wenn man eben nicht jede Kleinigkeit da anmerken würde. Aber gleichzeitig, das hast du auch gesagt, benutzen wir das ja als aktive, interne Fortbildungsmaßnahme, also wir schicken dann die Leute nicht zu irgendwelchen Fortbildungen,

  2. 44:24

    sondern wir nehmen die Stellen im Code, die auffallen, wo offensichtlich Probleme sind und nehmen uns da die Zeit so ausführlich wie nötig über die zugrunde liegenden Konzepte und Verbesserungsmöglichkeiten und sowas zu sprechen. Eine andere Downside, die ja bei ganz vielen anderen Firmen, wie ich das immer so höre, auch besteht, ist, dass das zum Teil lange dauert, bis ein Review gemacht wird. Also ich gebe was bei uns jetzt zum Beispiel in so einen Review-Pool und dann hat aber gerade keiner Zeit oder keiner nimmt sich die Zeit, sich das anzugucken und dann bin ich als Entwickler natürlich aufgehalten

  3. 44:57

    und ich will vielleicht sogar den nächsten Task machen und kann den aber nur auf diesem Feature-Branch machen, den ich jetzt gerade schon erstellt habe, wo ich ein PR für gemacht habe und dann muss ich den vielleicht sogar tatsächlich nehmen, muss darauf meinen neuen Feature entwickeln, dann kommen wir über Kommentare in einem anderen PR und dann kann man sich natürlich die wildesten Konstrukte vorstellen und ich glaube und ich glaube eine viel bessere Lösung als die Leute darauf hinzuweisen, dass PRs extrem wichtig sind, genauso wichtig wie jede andere Arbeit, die man macht und dass man auch so ein bisschen

  4. 45:25

    rewardet, wenn eben jemand PRs macht, da führt glaube ich kein Weg dran vorbei und wir forcieren das wirklich, dass wenn ein PR reinkommt, dass der im besten Fall eben nach kurzer Zeit und das heißt bei uns bestenfalls im Laufe von einer Stunde, aber allerschlimmstenfalls eigentlich innerhalb von einem Tag dann auch bearbeitet wird. Natürlich nur so lange wie keiner krank ist und wir haben es jetzt schon mehrfach bei uns im Unternehmen auch erlebt, ich bin krank oder ich habe Urlaub und ich bin jetzt nun mal qua unserer Unternehmensstruktur der technische Leiter, also auch ein bisschen für die Qualitätssicherung

Nachteil 2: Schlüsselpersonen 45:44–46:49

  1. 45:59

    verantwortlich und wenn ich dann eben nicht da bin, weil wegen Urlaub, dann heißt das, na gut, dieses letztendliche finale Review kann ich nicht machen und dann musst entweder du das machen oder die Kollegen oder es ist auf jeden Fall nicht so optimal oder wenn ich krank bin. Also man hat dann so ein Bottleneck und wenn das Bottleneck ausfällt, dann haben wir erstmal ein Problem. Wobei ich das Gefühl habe, dass das bei uns richtig gut funktioniert, weil immer wenn du nicht da bist, dann habe ich so das Gefühl, geben die anderen Entwickler und Entwicklerinnen nochmal mehr Gas beim Review und das ist ja jetzt nicht so,

  2. 46:33

    dass da jetzt eine riesen Lücke zwischen dir und allen anderen Programmierern ist, sondern wir haben ja auch sehr, sehr erfahrene andere Entwickler und für mein Dafürhalten klappt das auch sehr, sehr gut, wenn du nicht da bist. Aber ich will natürlich jetzt nichts von deiner Expertise wegnehmen. Zack, arbeitslos. Der dritte große Punkt, der immer wieder auffällt leider und wo wir auch noch keinen guten Umgang mitgefunden haben, ist, dass häufiger als mir lieb ist, grobe, strukturelle Probleme im Merge-Request erst ganz am Ende auffallen und wir da sicherlich vorher in der Planung deutlich mehr Anstrengungen

Nachteil 3: Aufgabenplanung 46:49–48:02

  1. 47:10

    unternehmen müssen, um das vorher abzufangen. Also wirklich, da kommt es vor, dass da wirklich nochmal große Teile des Reviews umgeschrieben werden müssen, weil da auf einmal irgendwelche grundlegenden Konzepte auffallen, die vorher nicht aufgefallen sind und das ist natürlich super frustrierend für alle, zum einen für die Kolleginnen oder den Kollegen, der den Review gebaut hat und jetzt alles nochmal umbauen muss, aber gleichzeitig auch für uns insgesamt, weil da natürlich super viel Zeit bei ins Land geht. Also da haben wir sicher noch eine riesen Baustelle bei uns als Firma, vorab die Tasks

  2. 47:46

    schon möglichst so zu planen, dass das nicht mehr vorkommt und solche Dinge im Vorhinein abzudecken. Wir machen es jetzt schon so, dass wir bei größeren Änderungen vorab eine Konzeptionsphase einschieben, also nochmal drüber sprechen, was wohl gemacht wird, bevor es gemacht wird, aber das ist auf jeden Fall noch eine Baustelle. Und trotzdem ist es für uns keine Alternative zu sagen, boah, komm, das wäre jetzt so aufwendig, wir winken es doch durch, weil dieser Qualitätsgedanke, der steht bei uns schon ganz oben und dann muss man halt nochmal ran, dann muss man es nochmal machen und da hilft alles nichts.

Fazit 48:02–51:01

  1. 48:16

    Ich glaube aber, dass diese Gespräche im Vorhinein, wo man kurz konzeptionell auch über vor allem größere Aufgaben spricht, dass das auch schon sehr hilft, sowas ein Stück weit abzustellen, aber natürlich kommt sowas trotzdem vor. Wir machen sicherlich nochmal einen Podcast darüber, wie wir Aufgaben stellen und was dazu gehört und sowas. Da bin ich selber gespannt drauf, weil da haben wir, glaube ich, noch viel Verbesserungspotenzial. Aber ich bin auch deiner Meinung, insbesondere mit dem Qualitätsstandard, den wir uns auf die Fahne geschrieben haben, ist so ein Merge Request in der Ausführlichkeit

  2. 48:45

    und dem Detailgrad und dem Umfang, wie wir es machen, unumgänglich. Hast du noch weitere Punkte, die du einwerfen würdest? Ansonsten haben wir ja jetzt schon die längste Folge, die wir jemals produziert haben und ich hätte ein kleines Fazit. Ne, gerne. Also ich bin mit meinen Aufzeichnungen tatsächlich durch. Ich hoffe, die Wichtigkeit und die Vor- und Nachteile sind klar geworden und man hat jetzt ein ungefähres Bild vor Augen, wie wir das machen und vielleicht die ein oder andere Inspiration gewonnen. Genau. Und als Fazit würde ich nochmal ganz kurz durch ein paar Punkte gehen, die wir jetzt genannt haben.

  3. 49:20

    Sicherlich nicht alle. Dafür hat man sich den Podcast angehört. Aber für mich gehören zu guten PRs unter anderem folgende Punkte. Die sollen möglichst klein sein, sodass die dann doch einigermaßen schnell bearbeitet werden können. Sie müssen einen Scope haben, also zum Beispiel einen bestimmten Task als Inhalt haben und darüber bestenfalls nicht hinausgehen. Ich sollte definitiv meinen eigenen PR nochmal reviewen, bevor ich ihn weitergebe. Ich sollte eine gute Beschreibung schreiben, einen Screenshot wenn irgendwie möglich, sodass ich dem Reviewer schon einen guten Hinweis geben kann, was ihn erwartet.

  4. 49:56

    Wir achten sehr auf Details. Das ist jetzt bei uns ein bisschen speziell, aber das würde ich trotzdem auch gerne so als Tipp mitgeben, dass man wirklich Attention to Detail ja auch machen soll. Dann wollen wir immer positiv sein. Wir wollen die Leute weiterbilden. Wir wollen dir was mitgeben. Wir wollen Wissenstransfer haben und das geht nur über eine positive Atmosphäre im PR. Und dann wollen wir als allerletztes die Entwickler und Entwicklerinnen wirklich ermutigen, nicht alles zu übernehmen, was im PR angemerkt wird, sondern wirklich eine aktive Diskussion zu starten. Das sind so für mich die wichtigsten Punkte.

  5. 50:33

    Genau, vielleicht abschließend einfach ein Mindset zu bekommen, dass so ein Merge Request nicht so eine lästige Aufgabe ist, die man mal eben abarbeiten muss, sondern eine Chance ist, für alle da dran zu wachsen und sich zu bessern. Das lassen wir als Schlusswort so stehen. Und Kay, ich freue mich auf die nächste Woche mit dir. Ich freue mich auch. Auf Wiederhören. Bis dahin.

03 — Weiterhören

Nebenan im Webcafé

Alle zwei Wochen montags eine neue Folge über Webentwicklung, Codequalität und die Art, wie wir zusammenarbeiten.

Alle 52 Folgen
04 — Feedback

Fragen an Felix und Kay?

Themenwunsch, Widerspruch oder eine Frage aus Deinem Projektalltag — schreib uns. Was uns erreicht, landet regelmäßig in einer der nächsten Folgen.

podcast [at] geenen-it-systeme.de