Transkript
Anja Kammer: Hallo und herzlich willkommen zum INNOQ Podcast. Mein Name ist Anja Kammer und ich habe mir heute zwei Gäste eingeladen. Einmal Martin Otten. Hallo.
Martin Otten: Hallo.
Anja Kammer: Und Martin Eigenbrodt.
Martin Eigenbrodt: Hallo Anja.
Anja Kammer: Gut, ja, wir sprechen heute über Code Reviews und Pull Requests und der Auslöser für dieses Thema war ein Artikel auf dem Blog von Martin Fowler. Martin, magst du mal erklären, worum es da ging?
Martin Eigenbrodt: Ja, sehr gerne. Dieser Artikel ist von einem Rouan Wilsenach. Ich bin mir nicht sicher bei der Aussprache. Und er stellt eine Variante vor, bei der man nicht immer Pull Requests macht, sondern das entscheidet. Und da gibt es drei Entscheidungsstränge. Die erste Variante nennt er ‚Ship‘. Das ist das, was am nächsten am Continuous Integration ist. Man tut seine Änderung einfach in den Main Branch. Und dann läuft das übliche automatisierte Testen und so durch und man tut dann gar nichts mehr und die Dinge gehen quasi so in Produktion. Die zweite Variante nennt er ‚Show‘. Da macht man einen Branch auf, merged den aber nach den automatisierten Tests selbst und danach zeigt man die Änderung. Wenn man also sagt: Ich habe hier was auf eine spannende Weise gemacht oder auf eine neue Weise, jemand sollte draufgucken, aber ich bin so optimistisch, dass ich selbst entscheide, das in Produktion zu bringen. Und die dritte Variante ist das, was für mich so die klassischen Pull Requests sind. Man erstellt ein Branch und wenn man fertig ist, stellt man den sozusagen zur Diskussion bzw zum Code Review und der wird dann von einer anderen Person gemerged, nachdem das eben angeguckt wurde. Man hat also diese drei Varianten, dass man sagt, ich mache entweder ‚Ship‘, ‚Show‘ oder ‚Ask‘. Irgendwie sind wir über diesen Artikel gestolpert. Den gibt es schon etwas länger und haben festgestellt, dass das so ein bisschen Anlass zu Diskussionen gibt und dachten deshalb: Sprechen wir mal darüber.
Anja Kammer: Ja, genau. Also mir war das gar nicht bewusst, dass man das anders sehen kann. Wenn man zehn Jahre lang am selben Software Projekt arbeitet, dann kennt man nur eine Art und Weise. Bei mir war es zum Beispiel so, mir wäre gar nicht in den Sinn gekommen, dass man beispielsweise Änderungen einfach so mergen kann. Aber wenn wir über Continuous Integration sprechen, dann redet man doch eigentlich auch darüber, oder? Dann ist es doch eigentlich dieses: Okay, ich merge. Und solange es nicht bricht, ist alles gut, oder?
Martin Otten: Ich fand ganz, ganz spannend an der Idee, dass ich mich entscheiden kann, je nach Fall in Projekten. Es ist nicht so, als ob ich im Projekt dann nur ‚Show‘ oder nur ‚Ask‘ mache, sondern ich kann mich entscheiden: Was mache ich denn jetzt in dem Fall? Ist es eine total simple Änderungen oder ist das was Komplexeres? Wie Martin schon gesagt hat. Und da hätte ich die Möglichkeit, genau diesen Continuous Integration Fluss, wo ich mich auf meine Tests verlassen kann, wo ich sage: Okay, hier läuft einfach die Pipeline durch, ich möchte kontinuierlich liefern, ich möchte nichts haben, was blockt. Das könnte dann der Standardfall sein, aber wenn es dann komplexere Dinge gibt, habe ich immer noch die Möglichkeit, dann entsprechend wirklich ein direktes Code Review und Pull Request zu haben und mich dann auf diese Sicherheit zu verlassen.
Martin Eigenbrodt: Mich hat der Artikel angesprochen, weil ich das Gefühl hatte, er formalisiert etwas, von dem ich dachte, es passiert sowieso immer automatisch. Die meisten Teams, in denen ich war, hatten formal die Regel: Es wird natürlich alles ge-reviewed. Diese Regel ist aber nicht technisch erzwungen worden. Das hat dazu geführt, dass genau das passiert, was im Artikel beschrieben wird. Jedes Entwickly hat für sich entschieden: Möchte ich das jetzt einfach so machen? Habe ich genug Vertrauen, dass das inhaltlich und technisch okay ist? Möchte ich einen Pull Request stellen? Oder zeige ich das? Und das war aber halt nie offiziell diskutiert. Und das finde ich ganz schön, dass das einfach mal formalisiert, was ich schon als gelebte Praxis wahrnehme.
Anja Kammer: Hmm, ja, interessant.
Martin Otten: Solche Diskussionen kenne ich zumindest auch aus Projekten. Wenn das die Regel gibt, es wird alles gereviewed, kommt irgendwann der Punkt, wo jemand dann doch mal was merged, was nur eine Zeile Config-Anpassung ist, wo man dann auch sagt: Ja gut, okay, dafür brauchte man es nicht. Aber es gab doch die Regel, dass alles gereviewed wird, dann hätte man es doch machen müssen. Und insofern finde ich das auch ganz gut, diese Option mal formalisiert zu haben.
Anja Kammer: Aber es gibt viele viele Meinungen dazu. Mich hat dieser Artikel krass erschüttert, weil ich gar nicht auf die Idee gekommen wäre, dass man Code einfach so merged. Ich hatte bisher noch keine Exposition zu einer anderen Arbeitsweise und wir haben auch bei einem Firmen-Event unsere Kollegys von INNOQ befragt, was sie denn von Code Reviews und Pull Requests halten und ich habe ein paar Kommentare mitgebracht und die würde ich gerne vorspielen.
Kollegy 1 Ich würde grundsätzlich immer Pull Requests oder Merge Requests machen, die gereviewed werden sollen. Mir fallen keine Ausnahmen ein.
Kollegy 2 Es sollte keine Dogmas geben zu Pull Requests und Reviews. Es ist alles Kontingent und kontextbezogen definiert. Meiner Meinung nach. Ich denke, es gibt Kontext. Man erkennt, wenn man das nicht braucht. Zum Beispiel Kontexte, wo das rückgängig zu machen trivial ist oder die Implikationen nicht so groß sind. Oder die Kosten ein Merge zu machen, ohne großes Review, nicht so groß ist oder überschaubar ist, dann kann man das einfach machen. Das gehört auch zu dem Thema kein Dogmatismus, sondern es sollte immer wieder neu verhandelt werden im Team und mit den Menschen, mit denen man arbeitet.
Kollegy 3 Ich finde aus der Sicht einer Pull Request stellenden Person ist es immer sinnvoll, wenn dieser Request dann auch mit Feedback beantwortet wird. Und wenn man sich jetzt bei der Teamarbeit fragt: Sollte das vielleicht immer so sein? Dann würde ich mir eine Projekt- oder Teamarbeit-übergreifende Absprache wünschen, die dann festlegt: Wenn es größere Arbeitspakete sind, dann müssen wir auf jeden Fall darüber sprechen und sonst eben nach Bedarf, wenn ihr unsicher seid, dann stellt ihr so einen Request. Und dann sollte der vielleicht auch zeitnah beantwortet werden. Die Rückmeldezeiten würde ich dann auch noch mal absprechen, sodass man nicht ewig wartet.
Anja Kammer: Das klingt so, als gäbe es schon Bedarf nach so einem Modell wie Ship, Show, Ask. Sodass ein Team erst mal tatsächlich explizit gemeinsam überlegt, in welchen Fällen Code Reviews und Pull Requests Sinn ergeben. Da gibt es schon Bedarf danach, habe ich das Gefühl.
Martin Otten: So ein Pull Request hat natürlich Kosten. Ich fand diesen Punkt mit den Kosten recht wichtig, weil ein Pull Request blockt erst mal, dann muss sich jemand das entsprechend anschauen, hat das vorher vielleicht noch gar nicht gesehen, muss sich dann da einarbeiten. Okay, worum geht es denn eigentlich in dem Change? Checkt das vielleicht auch aus, startet das, muss sich damit intensiver beschäftigen, wird dadurch aber natürlich auch aus einer anderen Tätigkeit rausgerissen und die Person, die auf den Pull Request wartet, das sollte zeitnah passieren, die fängt eventuell auch schon mal eine andere Arbeit an. So haben wir auf jeden Fall diese Kontextwechsel, die da mitkommen. Und ich denke auch, es gibt Fälle wo Pull Requests dennoch Sinn ergeben und wo man die sicherlich machen sollte. Pull Requests kommen eigentlich aus dieser Open Source Community und da habe ich auch ein Umfeld, wo ich ein geringeres Vertrauen habe in die Leute, die wirklich dann zum Projekt beitragen. Irgendwer macht einen Pull Request auf, schickt mir einen Patch für meine Software. Der Person vertraue ich natürlich nicht. In so einem Szenario macht es sicherlich Sinn. Da würde ich immer einen Pull Request machen, nicht Leuten erlauben, direkt in meinen Repo zu pushen. Und wenn ich so eine Situation auch im Projekt habe, wo vielleicht nicht so viel Vertrauen da ist, dann würde ich auch eher häufiger Pull Requests machen als in einer Situation, wo ich ein eingespieltes Team habe, Kollegen, denen ich vertraue und bei denen ein Standard Change einfach durchgehen würde und ich vielleicht nur ein bisschen hier und da mal den Stil vielleicht kommentieren würde.
Anja Kammer: Aber was ist denn Vertrauen? Wie bezeichnest du Vertrauen? Vertrauen in das Skill Set deiner Kollegys oder dass schon nichts schiefgehen wird? Was ist Vertrauen genau?
Martin Otten: Schon das Skill Set würde ich persönlich sagen. Das habe ich jetzt in meinem Open Source Projekt nicht. Oder wenn vielleicht ein neues junioriges Kollegy ins Team kommt, dann habe ich vielleicht auch noch nicht so das Vertrauen. Ich habe aber auch Vertrauen, dass man sich an Absprachen hält, an Coding Style Guides, dass man sich generell an die Architekturvorgaben hält, die man gemacht hat, sodass jetzt ein Change nicht so ein hohes Risiko hat, den er vielleicht hätte, wenn ich mit neuen Leuten zusammenarbeite, die ich noch nicht so kenne.
Martin Eigenbrodt: Das mit neuen Leuten zusammenarbeiten oder in einem neuen Team sein, ist für mich ein wichtiger Aspekt, denn es ist eher so, dass man einen gemeinsamen Stil entwickelt bzw. Entscheidungen abgleicht, was Design Prinzipien und Architektur innerhalb eines Projekts angeht. Und am Anfang ist das ja unsicher oder in der Schwebe und braucht auch mehr Kommunikation. Und da sind sicher Pull Requests richtig. Und wenn ich aber das etabliert habe und sich alle einig sind: Wir machen viele Dinge nach Schema F und ich habe das Gefühl, das ist jetzt wieder so ein Schema F Ding, dann sehe ich auch die Notwendigkeit nicht mehr so hoch. Was mir gerade eingefallen ist, ist, dass das Code Review nach einem Pull Request nur eine der Maßnahmen ist, die wir haben, um Qualität sicherzustellen. Es gibt tatsächlich Änderungen, die lassen sich schwer durch einen Code Review bewerten. Ein typisches Ding sind Versions-Updates von Bibliotheken. Dann erhöhe ich in irgendeinem Dependency Management Fall eine Zahl und da kann ich natürlich die Release Notes lesen, aber viel wichtiger sind da tatsächlich die automatisierten Tests, die sagen: Meine alten Erwartungshaltungen sind immer noch gedeckt damit. Und da ist der automatisierte Test viel wesentlicher, als dass jetzt jemand sagt: Okay, du darfst aus der 5 eine 6 machen.
Anja Kammer: Stimmt. Genau, oder was ist zum Beispiel mit Architektur Dokumentations-Update? Architektur Dokumentation wird auch häufig in Code Repositories, da wo auch der Applikations-Code drin ist gehalten. Brauche ich dafür einen Pull Request? Vielleicht sind es nur Typos, die ich fixen möchte. Wenn es so inhaltliche Änderungen sind, will ich dann vielleicht doch jemanden noch mal darüberschauen lassen, ob ich so korrekt gedacht habe. Bei mir ist es auch oft, dass ich so denke: Jemand sollte trotzdem darüberschauen, einfach damit zwei Leute wissen, dass da was passiert ist. Wenn man jemand fragt: Haben wir das dokumentiert? Wissen mindestens zwei Leute: Ja, das haben wir dokumentiert.
Martin Otten: Da ist für mich die Frage: Muss das blocken? Kann ich vielleicht auch einen Merge Request aufmachen und sagen: Hier, ich arbeite an was. Ihr könnt euch das gerne angucken, könnt mir Feedback geben und im Nachhinein kann ich immer noch sehen, da gab es eine Änderung und ich kann da noch reingucken. Gerade bei so einer Anpassung von der Doku, da passiert nichts Schlimmes, wenn ich die merge. Und da macht man dann noch mal einen neuen Merge Request auf, macht einen neuen Change und korrigiert es dann. Ich glaube, das ist schon wichtig, dass wir mehrere Stufen oder mehrere Möglichkeiten der Qualitätssicherung haben. Tests und auch die Möglichkeit, natürlich normale Code Reviews zu machen, wo ich das Gefühl habe, zumindest in den Projekten, in denen ich bin, findet das dann auch seltener statt, dass man sich dann noch mal hinsetzt und sagt: Wir gucken uns einfach noch mal den Code an, machen Code Review. Was haben wir da eigentlich gemacht in den letzten Monaten? Und wir stellen einfach mal unsere Lösung vor und diskutieren darüber.
Anja Kammer: Genau, das passiert selten. Das wäre dann dieser Modus. Es gibt drei Modi, Ship, Show, Ask in diesem Artikel und das wäre dann der Show Modus. Man macht eine Änderung, man macht ein Pull Request, damit es sichtbar ist, aber man merged trotzdem und man kann im Nachhinein Dinge zeigen oder im Nachhinein darüber reden. Aber macht das wirklich jemand, dass man im Nachhinein über Dinge redet? Passiert das bei euch?
Martin Eigenbrodt: Ich habe das schon erlebt. Ich habe mich an einem kleinen Detail gerade gestört. Ich glaube, Show heißt nicht unbedingt, dass es technisch ein Merge Request sein muss. Es kann auch ein Branch oder auch aus meiner Sicht auch ein Commit sein. Und das ist tatsächlich etwas, was ich in der Praxis oft erlebe, dass Menschen Änderungen machen und die deployen. Alles ist gut, es war vielleicht auch nur eine dringende Änderung, weil das ein Quickfix war und dann wird ein Link auf das Commit im Chat geteilt und gesagt: Guckt mal, ich habe das und das angepasst. Und das sind meistens recht kleinteilige Dinge. Und dann gibt es häufig einfach nur eine Emoji Reaktion darauf. Aber da habe ich schon das Gefühl, dass dann weitere Menschen draufgucken. Und das finde ich ganz gut, weil sie eben diesen Aspekt des Informationteilens drin hat und eben auch noch mal ein zweites oder drittes Paar Augen.
Anja Kammer: Gut, ein weiterer Kommentar von einem Kollegen geht auf das Sicherheitsgefühl ein. Er meint: Das Ziel ist nicht, sich selbstsicherer zu fühlen nach einem Code Review. Lass uns da mal reinhören.
Kollegy 4: Es kommt bei einer Code Review nicht darauf an, ob sich derjenige, der den Code geschrieben hat, sicher ist, sondern es kommt darauf an, dass derjenige, der den Code reviewed davon überzeugt wird, dass der Code sicher ist.
Anja Kammer: Genau. Wenn ich mir einen Code Review wünsche, sollte der Grund für diesen Wunsch dann sein, dass ich total unsicher bin, ob das überhaupt richtig ist oder funktioniert. Nach dem Motto: Ich bin mir unsicher, jemand anderes sollte ein Review machen.
Martin Otten: Ja, ich glaube, dass der Kollege meint, wenn ich ihn richtig verstanden habe, dass vielleicht die Person, die den Change macht, dass er das gar nicht unbedingt selber einschätzen kann oder einschätzen können muss, sondern dass die Person, die dann reviewed dann davon überzeugt werden muss, ob dieser Change denn sicher ist.
Anja Kammer: Naja, weil es muss einen Approval am Ende geben. Zumindest muss diese Person mit überzeugt werden.
Martin Otten: Genau, das würde auch bedeuten, dass ich als Person, die den Pull Request aufmacht, gar nicht selbst alleine entscheiden kann: Mache ich jetzt Ask, Show oder Tell? Für was entscheide ich mich denn da eigentlich? Und dann müsste das eigentlich das Team, wenn wir uns auf diesen Artikel beziehen, entscheiden.
Martin Eigenbrodt: Was da drin steckt ist, dass ein Code Review oder ein Pull Request immer eine Kommunikation in zwei Richtungen ist. Das eine ist, ich stelle den Request und kriege dann Feedback. Das andere ist aber, mit dem Request transportiere ich auch Informationen nach außen. Und ich glaube, was dieser Kollege gesagt hat, ist eben nicht: Ich mache das nur aus dem einen Wunsch, sondern eben auch aus dem anderen. Es müssen beide Kommunikationsrichtungen eigentlich bedient werden. Und wenn man jetzt sagt, das ist immer eine Team-Entscheidung, habe ich ein bisschen die Befürchtung, man macht das zum Bottleneck. Ich glaube schon, dass der verantwortliche Entwickly in der Lage ist zu entscheiden, wann es in welche Situation passt.
Anja Kammer: Ich glaube aber auch, wir haben gutes Reden, weil wir sind alle Senior Consultants. Wir haben mehrere Jahre Softwareentwicklungserfahrung. Aber stellt euch vor, es gibt, wie ich schon gesagt habt, neue Personen im Team, die vielleicht auch schon sehr viel Erfahrung haben, aber nicht konkret mit diesem Code. Manchmal ist der Code total messy, ist total von dem Kontext abhängig. Was total in Ordnung ist. Oder es gibt Neulinge in dem Coding Beruf oder Leute, die sich generell unsicher sind, weil das deren Art ist. Die, glaube ich, profitieren davon, dass sie zu jeder Zeit immer Code Reviews bekommen und dass sie sich vielleicht auch nicht schlecht fühlen, dass sie welche einfordern. Stellt euch vor, es gibt diese Regel: Nur in begründeten Ausnahmefällen machen wir Code Reviews, dann darfst du dir die Code Review wünschen. Und was ist, wenn diese Person unsicher ist und sich ein jedes Mal ein Code Reviews wünschen würde und sich dann aber schlecht fühlt? So eine Situation habe ich auch schon mal erlebt, dass die Leute sich sehr schlecht fühlen, weil sie sich gerne Code Reviews auch einmal zwischendurch wünschen.
Martin Eigenbrodt: Stimme ich dir voll zu. Ein Code Review einfordern zu können, muss immer erlaubt sein, auch gerade für meine eigenen Änderungen. Und ich finde es auch hilfreich zu sagen bei Menschen, die neu im Team sind oder neu im Code zu sagen: Das ist der Default. Tatsächlich ist das paradoxerweise ein Zeichen von Seniorität, früher um Hilfe zu fragen. Das ist eine Beobachtung, die ich gemacht habe. Dass gerade Leute, die sich recht unsicher sind, sich nicht trauen. Und Leute, die sehr souverän sind, vergleichsweise früh kommen und sagen: Pass auf, ich habe hier zwei Optionen. Ich bin total unsicher, ich habe mir das und das überlegt. Hast du eine Meinung? Und das muss man auch fördern und Code Reviews anbieten und dass sie immer eine Möglichkeit haben. Da bin ich ganz dabei.
Martin Otten: Ich finde auch, das ist auch ein Zeichen, dass man in einem guten Team arbeitet, wenn es da eine Vertrauensbasis gibt. Und nicht alleine vor sich hinarbeitet, oder die Erwartung ist, dass das alle schon alleine hinkriegen, sondern wenn wir schon am Team arbeiten, dann sollten wir auch kommunizieren und uns helfen und auf die Bedürfnisse aller eingehen.
Anja Kammer: Martin, wie du gesagt hast, es gibt Leute, die sich selbst vielleicht ein wenig überschätzen. Vor allem sind es Menschen, die eher wenig Erfahrung haben. Und dann diese Kurve von: Ich weiß alles. Und ich habe mitbekommen, ich weiß eigentlich gar nichts. Sie sind noch am Anfang dieser Kurve, sind sozusagen auf dem Peak und glauben, sie wüssten, dass sie alles richtig gemacht haben. Genau diese Personen würden dann eben auf Code Reviews eher verzichten wollen. Und dann ist schon wieder die Frage: Kann ich dann meinem Team sozusagen vertrauen, dass sie sich wirklich richtig einschätzen?
Martin Otten: Wir gehen auch gerne eher schlechten oder negativen Dingen aus dem Weg und ich bekomme eventuell Feedback. Dann sagt mir jemand, das war aber nicht so gut. Das fand ich nicht so toll und das vermeiden wir gerne. Ich hat nicht mit Code Reviews zu tun, aber zumindest bei Scrum oder agiler Entwicklung, den Retrospektiven. Da habe ich auch die Erfahrung gemacht, dass Leute in Teams sagen: Nee, wir brauchen das nicht, dass dann da eigentlich am meisten Redebedarf ist in den Retros, die sie eigentlich gar nicht machen wollten. Und so kann ich mir das auch bei den Pull Request und Code Reviews vorstellen, dass man dann lieber dem Feedback aus dem Weg geht und sagt: Nee, das ist jetzt mein Code, guckt da lieber nicht rein.
Anja Kammer: Ja, dazu habe ich auch einen guten Kommentar. Von einem anderen Kollegen. Spiele ich mal ab.
Kollegy 5: Eine ganz wichtige Erfahrung, die ich gesammelt habe bei Code Reviews ist etwas oder ist ein Konzept, das nennt sich Egoless. Das ist sehr spannend, wenn man das beim ersten Mal praktiziert. Es geht darum, dass man, wenn man der Empfänger eines Reviews ist, dass man wirklich ganz stark darauf achtet, sein eigenes Ego herauszuhalten. Der Code wird gereviewed, nicht man selbst. Und das ist eine der Sachen, die normalerweise den Leuten Reviews super unangenehm macht. Wenn man aber mit dieser Einstellung rangeht, dass man selbst überhaupt nicht kritisiert wird oder überhaupt auch nur bewertet wird, dann kann man die Ergebnisse eines Reviews sehr, sehr gut entgegennehmen.
Anja Kammer: Das ist im Grunde das, was du gesagt hast. Und ich stimme dem Kollegen und dir auch total zu. Und ich glaube auch, dass es der Grund ist, warum es eine Abneigung gegenüber Code Reviews oft gibt. Weil es unangenehm ist. Das wird als Bloßstellen wahrgenommen, Feedback zum Code erhalten, die man selbst produziert hat. Und da wird jeder Kommentar als Abwertung verstanden. Ich finde es dann auch sehr schwierig, dieses Problem zu lösen. Es ist interessant, oder? Die Softwareentwicklung ist schon auch ein Menschenproblem mitunter.
Martin Eigenbrodt: Ja, total. Und ich stimme dem Kollegen zu, dass das natürlich ein hehres Ziel ist. Egoless ein Code Review anzunehmen, ist aber natürlich nicht so leicht. Das ist menschlich ganz klar. Wenn ich jetzt so nachdenke, wie das emotional ist, ist das Gefühl, was ich am häufigsten habe bei Code Reviews das Ertappt fühlen. Das sind häufig Stellen, wo ich eigentlich wusste, das war jetzt ein bisschen schludrig. Oder ich habe den Fokus woanders drauf gelegt, ich war ein bisschen schnell. Da fühle ich mich dann schlecht. Es ist selten, dass es sich bei Diskussion wegen Kernentscheidungen schlecht fühle. Da kann ich dann gut technisch diskutieren, weil ich mich bewusst für einen Weg entschieden habe und kann den auch verteidigen oder eben diskutieren, ob der andere doch besser wäre. Aber es sind diese Ecken, wo man so darüber geschlunzt ist, wo ein bisschen Emotion auch reinkommt. Das ist glaube ich einfach Training, was wir alle ständig mit uns selbst ausmachen auch.
Martin Otten: Es ist auch nachvollziehbar, dass man sich dann da auch angegriffen fühlen kann. Man hat viel Arbeit reingesteckt. Das ist die Lösung, die man gefunden hat. Und dann kommt jemand und sagt: Du, das ist schön, was du gemacht hast. Aber guck mal hier und hier, das sollten wir anders tun. Wir müssen jetzt das, was du in den letzten drei Tagen gemacht hast, eigentlich noch mal neu machen. Und lass uns das noch mal vielleicht auch zusammen machen. Trotzdem ist das die Arbeit, die ich gemacht habe, auf die ich vielleicht auch entsprechend stolz bin. Und ich habe es halt so gut gemacht, wie ich kann. Und ich persönlich finde es halt auch schwer, dieses Feedback zu bekommen. Desto größer, stärker das Feedback sein muss, wenn man sagt: Eigentlich muss ich jetzt hier sagen, so können wir das nicht machen, wir sollten das noch mal bearbeiten, so können wir das nicht übernehmen. Das ist natürlich ein hartes Feedback, was mir auch schwerfällt, das entsprechend so zu verpacken, dass die Person sich dann nicht davon persönlich angegriffen fühlt. Ich versuche meistens noch irgendwelche positiven Aspekte herauszuheben und zu sagen: Okay, die Sachen fand ich nicht so gut, aber hier, das hast du schön gelöst. Um auch positive Dinge hervorzuheben.
Anja Kammer: Und ich glaube auch hier wieder von der anderen Seite. Menschen, die neu sind oder sich nicht so seniorig fühlen, begründet oder unbegründet, haben eher die Tendenz, zumindest habe ich das beobachtet, dass sie sehr froh sind, Code Reviews zu bekommen und auch, dass da super viel Feedback ist. Sie freuen sich, dass die Fehler überhaupt gefunden wurden, damit sie nicht der Grund sind, dass irgendwann das System down ist, weil sie ein Bug eingebaut haben. Lieber wird der Bug schon vorher gefunden statt später, wenn das System down ist, weil sie so unsicher sind. Ich weiß noch, als ich angefangen habe mit der Programmierung, habe ich durch Code Reviews Programmieren gelernt. Ich habe den Code meiner Senior-Kollegen gereviewed und fand das super spannend. Ich konnte viele Fragen stellen und es war auch total in Ordnung, weil alle wussten: Die Anja lernt und die Anja hat Fragen. Also lassen wir sie mal. Da habe ich sehr viel gelernt. Auch von der Betrachtungsweise. Und da müssen natürlich auch die Kollegys einsehen: Okay, das ist jemand, der stellt Fragen nicht aus Kritik, sondern einfach weil die Person lernen möchte, wie das funktioniert und warum das so funktioniert.
Martin Eigenbrodt: Da finde ich auch als Review gebende Person kann man häufig viel lernen. Häufig sind es gerade die, ich sage mal unschuldigen Fragen, die einen erst mal sprachlos machen. Dann muss man mal reflektieren. Ich habe das immer so gemacht und es ist total etabliert und anerkannt das so zu machen und dann diesen Schritt zu gehen, das zu begründen, mal darüber nachzudenken finde ich oft sehr wertvoll, weil man selbst einfach ins Erklären und Denken kommt und das hilft auch ungemein zu sortieren.
Martin Otten: Und Anja, das, was du gesagt hast, auch mit der Sicherheit. Ich glaube, das ist auch ein wichtiger Aspekt, der auch mit der Sicherheit von der Person, die gereviewed wird, zu tun hat, und von der Firmenkultur, die vorherrscht und von Vertrauen im Team. Es gibt auch Leute, das ist mein Eindruck, wenn man in ein neues Team reinkommt, wo Leute dabei sind, die in einer bestimmten Firma vorher gearbeitet haben, wo sie auf die Finger bekommen haben, wenn sie da was falsch gemacht haben. Und es gibt sehr oft: Das ist falsch. Wieso hast du das nicht anders gemacht? Dass es auch direkt ein Vorwurf ist. Das ist dann kein Review auf Augenhöhe. Und wenn man sich dann mal daran gewöhnt hat, dass das eigentlich was Positives ist, das Feedback und dass es einem nicht negativ ausgelegt wird und das Vertrauen aufgebaut hat, dann denke ich, wird das auch besser. Und ich glaube, in dem Fall hat das gar nicht so viel mit Ego zu tun, sondern hat mit der Unsicherheit und dem Vertrauen im Team, dass man dieses Feedback auch annehmen kann, ohne sich angegriffen zu fühlen.
Anja Kammer: Ja, guter Punkt. Du hattest auch erwähnt bei einem Code Review. dass es eine gute Idee ist, dass man sagt: Lasst uns das doch mal gemeinsam machen. Gemeinsam machen, da kommt für mich Pair-Programming in den Sinn. Und wir haben auch Kommentare zu Pair-Programming von den Kollegys vom Firmen Event.
Kollegy 6: Ich finde Code Reviews toll, aber am besten funktionieren sie meiner Meinung nach, wenn man Pair-Programming, Mob-Programming macht und andere Formen benutzt, die nicht den Flow irgendwie blockieren, dass man nicht Ewigkeiten drauf warten muss, dass irgendjemand Feedback gibt zu irgendwas.
Anja Kammer: Ja, da haben wir wieder dieses Wartezeiten vermeiden Ding. Mit Pair-Programming könnte ich Wartezeiten vermeiden, dann fühlt man sich gemeinsam vielleicht sicherer, dass das, was man gemeinsam erarbeitet hat, korrekt ist.
Martin Eigenbrodt: Ich finde es spannend. Ich programmiere gerne im Pair. Aber gerade das mit den Wartezeiten erlebe ich anders. Bei Pair Programming erfordert, dass ich Zeit-synchron arbeite und je stärker wir alle im Homeoffice sind, desto schwieriger wird das in meiner Wahrnehmung. Ich habe das häufig, dass ich vor den Kollegen anfange zu arbeiten und früher in die Mittagspause gehe und so. Und wenn ich alles im Pair tun wollte, käme ich gar nicht mehr voran. Und da sind eben Code Reviews oder Merge Requests in ihrer asynchronen Art und Weise eben genau hilfreich, um mich nicht zu blockieren.
Martin Otten: Wobei ich mir auch gut eine Mischung vorstellen kann. Ich mache auch sehr gern Pair-Programming. Ich persönlich glaube auch, da bin ich am produktivsten und ich kann mir aber eine Mischung vorstellen, dass man anfängt an einem Thema zu arbeiten und sich dann aber immer mal wieder abstimmt. Dann kann man auch früher anfangen, früher in die Mittagspause gehen. Man hat aber vielleicht trotzdem zwei, vier Stunden am Tag, an dem man gemeinsam arbeiten kann. Und das ist natürlich auch eine Möglichkeit noch der Qualitätssicherung, die wir eben noch gar nicht genannt haben. Ich glaube aber auch, und eine Frage, die ich mir gestellt habe bei dem Artikel ist, dass dieses, wenn ich eigentlich nach dem Pull Request frage und ich habe diesen Fall, dass ich sage: Hier habe ich eine komplexe Änderung, da brauchen wir auf jeden Fall einen Pull Request. Ob das nicht eigentlich der Fall ist, wo ich sagen könnte oder müsste: Ich mache das dann direkt im Pair Programming. Oft wird dann auch gesagt von Leuten, die gar nicht so gerne Pair Programming machen. Einfache Änderungen, Dinge, die ich jeden Tag mache. Das muss ich nicht im Pair Programming machen. Und da frage ich mich: Sind das dann nicht die Fälle, wo ich sage: Ich brauche auch gar keinen Merge Request? Und die Fälle, wo ich sage, ich bräuchte einen Pull oder Merge Request sind die, wo ich dann per Pair Programming machen könnte.
Anja Kammer: Pair Programming ist aber wieder eine soziale Interaktion und manche Menschen haben andere Vorlieben. Ich bin zum Beispiel jemand, ich mag Pair Programming nicht. Ich habe das mal in einem frischen Projekt gemacht. Ich glaube drei Wochen lang, jeden Tag gemeinsam acht Stunden lang Pair Programming gemacht. Das war die anstrengendste Zeit, die ich jemals erlebt habe. Es war sehr anstrengend der Person zum Beispiel zu folgen, weil die zum Beispiel sehr viel schneller war. Und es war anstrengend, auf der Seite zu sein, die schreibt, weil ich mich immer beobachtet gefühlt habe und dann so, ich möchte nicht sagen Blackouts hatte, aber schon in der Art und Weise total dumme Fragen gestellt habe und überhaupt gar nicht mehr wusste, wo ich weitermachen würde. Hätte ich alleine gearbeitet, wäre das für mich total normal gewesen und total klar, weil es weniger Stress erzeugt hat. Dieses Pair Programming erzeugt bei mir Stress, obwohl ich mit meinen Kollegys super klar komme, es auch ein freundschaftliches Verhältnis ist und auch Vertrauen da ist. Manchmal funktioniert es gar nicht.
Martin Eigenbrodt: Ich bin voll bei dir, dass es total anstrengend ist und acht Stunden am Tag Pair Programming wäre für mich nichts, sagen wir mal so. Es gibt Kollegen, die das versuchen, sehr intensiv im Mob zum Beispiel das zu machen. Das könnte ich nicht. Und es gibt noch so ein Aspekt. Der Martin hat das gerade gesagt, dass wenn man an dem Punkt ist, wo man Entscheidungen treffen muss, dann ins Pair zu gehen, das finde ich schwierig, weil gerade die komplexen Entscheidungen muss man eigentlich vorbereiten. Ich habe oft das Gefühl, ich kann eigentlich in einem Call oder in einem Pair nicht richtig denken, komplexere Sachverhalte, sondern die muss ich für mich aufbereiten, vorbereiten. Dann kommt da ein Schmierzettel raus oder vielleicht auch eine Outline, irgendwas Strukturiertes. Das kann ich teilen und dann kann ich darüber diskutieren. Aber ich brauche auch immer so eine Zeit, um mich selbst erst mal zu sortieren. Das ist, wenn ich einen Code runterschreibe, wo klar ist, wo es hingeht, kann ich gut pairen. Aber wenn es um grundlegende Entscheidung gibt, muss ich erst mal die Chance haben, alleine zu reflektieren.
Anja Kammer: Schließe ich aber auch nicht aus. Man kann davor sich Zeit nehmen und dann, wenn man bereit ist, ins Pair Programming gehen.
Martin Eigenbrodt: Auf jeden Fall.
Martin Otten: Ich denke auch, dass es wichtig ist. Es gibt Optionen. Wenn man Pair Programming nicht mag, kann man natürlich auch ein Pull Request aufmachen. Dann wäre das die asynchrone Variante, wo ich Text schreibe. Natürlich kann ich so ein Pull Request natürlich auch reviewen, indem ich das zusammen mache und dann vielleicht einen Call aufmache und gemeinsam auf einen Code drauf gucke. Aber da habe ich auf jeden Fall diese Möglichkeiten. Für mich wäre es auch in Ordnung, nur dann Pair Programming zu machen und dann den Pull Request wegzulassen. Wobei wir, glaube ich, dazu auch noch eine andere Meinung haben.
Martin Eigenbrodt: Ja, mich zum Beispiel. Aber du bezogst dich jetzt auf die Kollegin oder Kollegen vom Event? Dann hören wir doch da erst mal rein.
Kollegy 7: Ich denke, dass Code Reviews eine der wichtigsten Sachen sind, um Software Qualität zu sichern, weil alle Leute machen Fehler, wenn sie alleine arbeiten. Und ich glaube auch, dass wenn man zum Beispiel paired, dass man trotzdem noch jemand außerhalb dieses Teams braucht, der ein Review durchführt, weil man oft zusammen in so ein Group Think reinkommt und dann gemeinsam Fehler passieren, die die Person, die das reviewed bemerken würde.
Martin Eigenbrodt: Ja, das ist tatsächlich genau das, was ich meinte. Wenn ich gut im Pair programmiere, dann groove ich mich ein bisschen aufeinander ein und man verfolgt so eine gewisse Richtung. Man hat immer auf bestimmte Dinge einen Fokus und verliert vielleicht andere Dinge aus den Augen. Und dann ist auf jeden Fall noch mal ein Review nötig. Ich kann mir auch vorstellen, dass eine der mit programmierenden Personen das Review macht, aber vielleicht am nächsten Tag, also mit ein bisschen Abstand. Aber es gibt so eine Betriebsblindheit, in die man eben nicht nur alleine kommen kann, sondern auch durchaus im Pair. Ich habe Teams erlebt, wo es hieß, was im Pair gemacht wurde, das muss nicht gereviewed werden. Das würde ich so allgemein ablehnen wollen. Und grundsätzlich kann ein Review gerade auch dann sinnvoll sein.
Martin Otten: Ich habe aber auch schon erlebt, dass dann trotzdem Leute gesagt haben: Das haben wir im Pair gemacht. Es wäre aber trotzdem gut, wenn da noch jemand drauf guckt.
Anja Kammer: Also sollte man bei jedem Pull Request, egal ob allein geschrieben oder im Pair, noch entschieden werden, ob es jetzt gereviewed werden sollte von einer anderen Person oder nicht.
Martin Eigenbrodt: Sehe ich so. Das ist unabhängig davon.
Anja Kammer: Also gibt es keinen Unterschied.
Martin Otten: Ich glaube, das wäre generell eine gute Idee. Natürlich können beim Pairen Fehler passieren, da kann was durchgehen. Das kann aber auch bei einem Pull Request passieren. Ich weiß nicht: Wie intensiv guckt die Person darauf? Wie ist die mit der Fachlichkeit vertraut, was da umgesetzt werden soll? Vielleicht habe ich da bestimmte Business Regeln, die umgesetzt werden. Schaut die nur kurz drauf, weil sie wenig Zeit hat? Sagt: Oh nee, die Person und Teamkollege wartet drauf. Ich muss das schnell machen. Ich gucke darüber. Ja gut, keine wirklichen Schnitzer im Code. Daumen hoch, alles super. Kann weitergehen. Oder habe ich mich wirklich so sehr damit auseinander gesetzt? Oder kann das überhaupt aufgrund von meiner Kenntnisse von den Change, dass ich dann diese Fehler auch finde?
Anja Kammer: Ein Paradebeispiel ist da dieser Approval Kommentar: Looks good to me. Da habe ich immer so ein schlechtes Gefühl, wenn jemand sagt: Looks good to me. Oder noch schlimmer wenn jemand sagt: Ich habe davon keine Ahnung, ich vertraue dir dann. Da habe ich ein ganz schlechtes Gefühl.
Martin Eigenbrodt: Das ist auf jeden Fall nicht hilfreich als Kommentar. Ich glaube, wir sind an dem Punkt, ganz wichtig ist auch, welche Form so ein Pull Request hat. Welche Größe und welche Komplexität. Und da gibt es auch dieses Paradoxon: Je kleiner ein Pull Request ist, desto mehr Feedback bekommt man. Und bei einer bestimmten Größe bekommt man eben leider oft nur noch ein ‚Looks good to me‘, was eigentlich bedeutet: Es ist zu groß. Keiner kann nachvollziehen, was du gemacht hast.
Anja Kammer: Ich finde ein großes Thema ist da auch Verantwortungsgefühl. Habe ich mehr Verantwortung für meine Arbeit, wenn es keine Pull Requests mehr gibt oder wenn es Requests gibt und diese dementsprechend auch Approval, kann ich dann meine Verantwortung, die ich für meinen Code normalerweise trage, dann auch auf die Schultern des Reviewende nverteilen? Wie seht ihr das?
Martin Otten: Ich finde, auch das ist für mich ein wichtiger Aspekt. Eigentlich finde ich gefährlich, diese Kombination aus: Ich gebe Verantwortung ab. Ich bin nicht mehr allein verantwortlich dafür, weil es wird noch gereviewed. So ein bisschen wie das Qualitätssicherungs-Team, dem man den Change am Ende noch mal gibt, damit die dann testen, wo dann die ganzen Tester sitzen. Da gibt man auch diese Verantwortung ab und sagt: Ja gut, da guckt ja noch niemand darüber, ist doch nicht so schlimm. So ist das eigentlich mit dem Pull Request dann aus meiner Sicht auch. Und wenn die dann noch nicht mal richtig dann gemacht werden und dann einfach ein ‚Looks good to me‘ da steht oder: 'Ja, passt schon, ich vertrau dir. Dann hat es nicht mehr viel Wert. Aber die Person, die den Pull Request stellt, hat trotzdem ein Stück Verantwortung abgegeben von dem Ergebnis, weil sie hat es nicht alleine entschieden, dass das in Produktion ist.
Anja Kammer: Andere Meinungen von euch zum Verantwortungsgefühl.
Martin Eigenbrodt: Naja, was Martin sagt, trifft sicher zu, dass man das Gefühl hat, sie abzugeben. Dass es nicht ideal ist, ist aber auch klar. Eigentlich will man als Team die Verantwortung trotzdem behalten und muss schauen, wie man diese Situation möglichst vermeidet. Dass man Config Reviews bekommt, die wertvoll sind. Eine Erfahrung, die ich da mal gemacht habe, war in einem Team, wo viele meiner Merge Requests ganz viele kleine Kommentare hatten, die sich auf so Dinge wie Formatierungen bezogen oder was ich zum Beispiel auch geplanked habe, also doppelte Leerzeichen an vielen Stellen. Und tatsächlich haben wir dann was gemacht, was ich vorher total abgelehnt habe. Wir haben nämlich dann Tools eingebaut, die so was überprüft haben und vorher abgelehnt haben. Und das hat dazu geführt, dass dann die menschlichen Reviews sich tatsächlich wieder auf Inhalte konzentrieren konnten, weil dieser technisch erledigbarer Kleinkram vorher weg war. Und das habe ich als sehr entlastend empfunden, dass man dann wieder über Sachen diskutieren konnten, die wirklich sonst auch durch die Lappen gegangen wären.
Anja Kammer: Ich habe mal mit einer Kollegin gesprochen, Joy Heron, und die hatte da ein spezielles System für Code Reviews, und zwar in Iteration. In der ersten Iteration hat sie nur auf den Implementationsinhalt, auf die Logik geschaut, ob das was im Ticket steht auch genau so implementiert wurde, ob die Logik korrekt ist. Und ich kann mich leider an die anderen Iterationen nicht mehr erinnern. Aber ganz zum Schluss kam dann so was wie: Code-Formatierung und Schönheit und so weiter. So könnte man sich natürlich auch darauf konzentrieren. Zuerst einmal das Wichtigste zu besprechen und sich nicht an Kleinigkeiten aufzuhängen, die dann irgendwie den Pull Request total aufblähen. Aber ich fühle mich auch ertappt. Ich mache das natürlich immer sehr gerne. Auf Kleinigkeiten zuerst zu achten, weil mich das ein bisschen wahnsinnig macht, wenn ich zu viele Leerzeichen sehe, da muss ich mich in Zukunft auch mal zurückhalten. Ich muss es mal so machen wie Joy.
Martin Eigenbrodt: Diese Idee mit Iterationen finde ich sehr gut. Ich mache so was auch, aber es wäre vielleicht mal sinnvoll, das tatsächlich zu formalisieren. Was ich oft mache, ist, dass ich erst mal versuche grob nachzuvollziehen: Was ist denn passiert? Das zu zerlegen mit, wo wurden Klassen eingefügt, ein Pattern angewendet, Dinge umbenannt? Und das ist leider so, dass diese Divs, die wir angucken auf Zeichenebene eigentlich gar nicht den Inhalt transportieren. Wenn ich ein Package umbenenne in Java, dann habe ich an Hunderten von Stellen möglicherweise Änderungen und die erzeugen unheimlich viel Rauschen in so einem Div und sind aber eigentlich alle mit einem Satz beschrieben. Und das muss man im Grunde dann so reverse engineeren. Und da kann es sehr hilfreich sein, wenn das vorher schon getrennt wird. Was ich gerne mache, ist, dass ich für ein Feature mehrere Merge Requests mache, zum Beispiel eben vorbereiten oder so Refacturings, wo ich dann dran schreiben kann, dass sich eigentlich nichts geändert haben soll im Verhalten der Software. Ich kann schreiben was ich gemacht habe, um das dann vielleicht zu erleichtern, das zu reviewen.
Martin Otten: Wie sieht denn für euch generell gutes Feedback aus, für jemand, der das Review durchführt? Sind das nur Kommentare oder wäre es für euch auch in Ordnung, direkt eine Codeänderung vorzuschlagen und zum Beispiel zum Merge Request. Nach dem Pull Request direkt noch mal einen Pull Request zu machen und zu sagen: Ja, schaut mal, ich habe das hier noch mal ein bisschen refactured. Ich habe direkt die Leerzeichen rausgenommen. So hätte ich das jetzt gelöst. Wollen wir darüber mal reden?
Anja Kammer: Ja, spannend. Mache ich auf jeden Fall. Mache ich aber nur, wenn es sehr schwer ist, es zu erklären oder wenn ich zeigen möchte, dass es so besser ist und man es eben nur visuell zeigen kann. Weil diese Tools wie GitHub und GitLab zeigen einem dann schön visualisiert den Div. Aber für Leerzeichen würde ich es nicht machen. Martin, du?
Martin Eigenbrodt: Für so kosmetische Dinge, Leerzeichen, Typos und so push ich das einfach oft selber auf dem Branch noch. Ich denke, da kann keiner was gegen haben. Da mache ich das dann so. Dann gibt es aber die Änderung, wo man vielleicht ein anderes Vorgehen vorschlägt oder was grundsätzlich anders haben will. Und da ist immer so ein bisschen die Frage, wie viel Zeit man auch selbst dann reintun will. Wenn man auf einer Wellenlänge ist mit der anderen Person, dann reicht es oft zu sagen: Pass auf, da und da hätte ich das vielleicht so und so designed. Wenn aber da so ein Verständnisgefälle ist, muss man oft tatsächlich den Code zumindest grob produzieren, um darüber reden zu können, was man denn eigentlich vorschlägt. Dann ist man schon sehr nah an einem Merge Request für den Merge Request. Das ist tatsächlich was, was ich sehr unterschiedlich handhabe, je nach Situation.
Anja Kammer: Also machst du schon einen eigenen Commit dann in den Branche rein, hast du ja gesagt? Das, was ich vorher erwähnt habe, war diese Vorschlagsfunktion von GitHub und GitLab, dass man einen Vorschlag macht. Und wenn die andere Person diesen Vorschlag gut findet, dann kann das gleich committed werden von der UI aus.
Martin Eigenbrodt: Stimmt, das habe ich auch schon verwendet. Ich bin da noch nicht so daran gewöhnt, aber ich glaube, dass es eigentlich gut ist für so kleine Dinge. Manchmal bin ich ein bisschen langsam mit der Adoption gerade von so UI-basierten Tools. Da brauche ich ein bisschen länger, um warm zu werden.
Anja Kammer: Das nächste Thema ist Qualitätssicherung. Da haben wir ein paar Kommentare von den Kollegys von dem Firmen Event.
Kollegy 8: Ich finde Code Reviews geil und die machen mir Spaß, weil ich da immer jede Menge lerne, was man alles falsch machen kann und Leuten dabei helfen kann, die Dinge anschließend richtig zu machen.
Kollegy 9: Ich mache ständig Fehler, deshalb finde ich Code Reviews total gut und deshalb mag ich auch Pull Requests.
Anja Kammer: Wir haben zwei Aspekte gehört. Einerseits möchten Reviewer gerne Fehler der anderen korrigieren können und finden es deswegen richtig gut, so Pull Requests etwas generell auch zu haben. Aber dann gibt es auch Entwicklerinnen, die selbst wissen, dass sie Fehler machen. Und die müssen nicht mehr total unsicher sein. Den Kollegen, die wir gerade gehört haben, der ist einer der erfahrensten Consultants bei INNOQ, das kann man schon sagen. Und selbst er sagt von sich, er würde ständig Fehler machen und das ist auch das, was du gesagt hast, Martin. Das ist auch ein Zeichen von Seniorität, wenn man sich selbst nicht ganz so viel zutraut und mehr nach Feedback fragt. Und hier ist ein Code Review ein Werkzeug für Qualitätssicherung. Und das kann verschiedene Bedürfnisse geben.
Martin Eigenbrodt: Das sind eigentlich die positivsten Momente, wenn man so einen Code Review empfängt und kriegt einen Weg gezeigt, wie es vielleicht viel einfacher oder eleganter ist. Manchmal ist es einfach Wissen, das man nicht hat. Manchmal gibt es eine Bibliothek oder eine Funktion in der Bibliothek oder ein anerkanntes Muster oder man hat einfach blinden Fleck irgendwo und dann kann das wie so eine Erleuchtung sein in dem Moment so: Woah, Mensch, das ist ja toll, jetzt kann ich hier drei Seiten Code wegschmeißen und es ist total super nachvollziehbarer. Vielen Dank. Das ist sehr schön, wenn man so ein Code Review kriegt, wo einem dann echt so ein Licht aufgeht.
Martin Otten: Aus meiner Sicht würde ich das noch mal ein bisschen differenzieren wollen, die sagen: Es kommt darauf an. Ich finde eigentlich immer noch diese Idee aus dem Artikel, den wir so als Start genommen haben. Ich kann es ganz gut. Klar, wir alle machen Fehler und ich mache auch ständig Fehler. Die Frage ist für mich: Wie gravierend sind diese Fehler? Sind das Fehler, die würden durch die Tests abgefangen. Das wären wahrscheinlich die Dinge, die ich schon haben will, dass ich sicher bin, wenn ich das deploye, dann geht nichts kaputt. Oder sind es Fehler, wo ich sage: Okay, ich hätte das anders lösen können. Da gibt es eine schönere Lösung für. Das ist jetzt zu kompliziert, wo wir auch ein nicht lockendes Review vielleicht haben, wo wir sagen: Gut, wir gehen einfach dann noch mal durch, alle ein, zwei Wochen, die machen nochmal ein nachgelagertes Review und dann kann ich das Feedback immer noch bekommen, kann davon lernen, kann von anderen lernen und kann immer noch den Code dann verbessern. Und ich finde, es hängt auch noch davon ab, in welcher Projektsituation ich mich befinde. Sind wir wirklich in einem Projekt, wo wir Continuous Integration machen können, wo wir eigentlich täglich oder mehrfach am Tag, vielleicht sogar idealerweise einen Release machen könnten? Oder sind wir in einer Situation, wo wir eh nur alle drei Wochen, drei Monate oder alle halbe Jahre an Release machen? Da kann ich mir vielleicht auch einfach ein bisschen mehr Zeit lassen und regelmäßiger Code Reviews machen. Da ist es auch nicht so tragisch, dass ein Pull Request dann erst mal den Fluss blockt.
Martin Eigenbrodt: Ich finde ganz spannend, was du gerade gesagt hast, dass es eigentlich nicht nur um Fehler geht, sondern ich glaube, es gibt verschiedene Arten von Feedback. Fehler sind eigentlich gar nicht so dramatisch, weil die schnell auffallen, idealerweise auch durch Tests und auch klar ist, was richtig und was falsch ist meistens. Wenn ich sich nicht so verhält, wie ich das erwartet habe. Und das andere sind diese etwas weicheren Dinge, so Design-Entscheidungen, die erheblich Einfluss auf die Lesbarkeit und die Wartbarkeit von Software haben. Und auf lange Sicht sind „Fehler“ auf dieser Ebene viel dramatischer, sind aber auch viel schwerer zu finden und zu diskutieren. Und ich glaube, ich stimme dir zu, dass das aber auch eine Klasse ist von Fehlern, die gar nicht unbedingt gefunden werden muss, bevor es gemerged wird. Für die Dinge wäre dieses Show vielleicht okay. Das geht jetzt erst mal, aber wir müssen da noch mal darüber nachdenken. Ich glaube, wenn wir dann das nächste Feature bauen wollen, passt das nicht mehr. Wir müssen das noch mal ändern.
Martin Otten: Ich glaube, das sind auch nicht unbedingt architekturelle Fehler. Das sind noch Sachen, die sich wahrscheinlich so einschleichen, die vielleicht auch gar nicht bei einem einzelnen Pull Request gefunden werden können, weil es immer so ein kleiner Ausschnitt ist. Und ich glaube, deswegen können Pull Requests auch nicht generell Code Reviews ersetzen, weil man dann später noch mal in den Code reinguckt und schaut sich alle Änderungen an, insgesamt die letzten Wochen oder Monate. Dann stellt man fest: Oh, hoppla, wir bewegen uns in eine Richtung, das ist nicht gut. Da sollten wir noch mal darüber nachdenken, ob das überhaupt der richtige Weg ist. Und ich glaube, das fällt wahrscheinlich bei so einem kleinen Pull Request dann gar nicht auf.
Anja Kammer: Ja, das stimmt, fällt aber noch weniger auf, wenn man wenig Pull Requests oder generell Code Reviews macht. Du hattest auch vorhin gesagt, wenn man mehr Zeit hat, macht man vielleicht auch mehr Pull Requests und Code Reviews. Und wenn man weniger Zeit hat, eben nicht. Aber das finde ich schwierig, weil ich finde, man sollte sich nicht davon drängen lassen, nur weil etwas schnell fertig werden muss, dass man dementsprechend Abstriche macht, was man sonst gemacht hätte. Und da hätte ich tatsächlich die Befürchtung, dass Qualität dann auch sinkt. Und ich finde, du hast recht, dass in Pull Requests man vielleicht gar nicht sieht, nur für ein Ticket, ob es da irgendwie ein Architekturproblem gibt. Aber wie oft schauen wir uns denn die Architektur von dem Code überhaupt noch mal gemeinsam an? Klar, das sollte man vielleicht tun, aber wie oft machen wir das? Wir haben das Gefühl, dass wir immer Tickets kloppen müssen. Wir müssen schnell sein. Wir müssen schnell Wert liefern. Nimmt man sich überhaupt genug Zeit, sich mal die Architektur genauer anzuschauen? Und was ist, wenn das Kind schon in den Brunnen gefallen ist? Kann man dann dem Product Owner oder je nachdem wer das entscheidet, dann sehr einfach mitteilen: Ach übrigens, wir haben da seit ein paar Wochen Quatsch gemacht. Wir müssen das noch mal aufrollen. Ach übrigens, du wirst keinen weiteren Wert bekommen. Das ist nur eine Architekturänderung, die an der Funktionalität gar nichts ändert. Kann man das gut argumentieren?
Martin Otten: Ich möchte einmal noch mal auf die Qualität eingehen. Ich bin nicht der Ansicht, dass man die Qualität vernachlässigt, wenn man in einer Situation ist. Weil wenn wir regelmäßig, wirklich regelmäßig deployen, dann stimmt eigentlich das, finde ich, was Martin sagt. Wir stellen auch fest: Oh, es funktioniert nicht. Wir können aber auch schnell diese Fehler korrigieren. Wenn wir in einem etwas langsameren Release Zyklus sind, dann sind wir meist nicht gewohnt, dann schnell die Dinge zu korrigieren. Dann haben wir vielleicht nicht diese kurzen Raumtrittzeiten. Zudem habe ich noch andere Maßnahmen wie Tests, dass ich feststelle, dass ich sicherstellen sollte, es geht jetzt nichts Schlimmes in Produktion, sondern es sind die kleinen Dinge, die nicht so tragisch sind, wo ich sagen kann: Die kann ich später dann auch korrigieren. Das andere, was du jetzt gesagt hast, ist natürlich generell eine Diskussion mit dem Wert. Das ist immer schwierig dann entsprechend dem Product Owner oder dem Business natürlich zu verkaufen, wo der Wert darin liegt. Und ich finde es wichtig, da aufzuzeigen, dass wenn man das jetzt nicht macht, dass das genau wie eine Wartung bei einem Auto ist, eine Reparatur, die man vor sich herschiebt, dass das höhere Folgekosten nach sich zieht und dass es auch einen Wert hat, jetzt frühzeitig da noch mal Fehler zu korrigieren oder Dinge umzubauen, um diese Folgekosten zu vermeiden.
Martin Eigenbrodt: Ja, wir haben bisher oft hier Pull Requests und Code Reviews ein bisschen synonym benutzt. Und was Anja jetzt aber gerade aufgezeigt hat, ist, dass es vielleicht verschiedene Dinge sind, dass die klassischen Merge Requests mit Reviews sind vielleicht notwendig, aber nicht hinreichend, sondern ich brauche auch Gelegenheit, mal auf das Ganze zu gucken. Ich kann das dann vielleicht Architektur Review nennen oder eben großes Review, aber da muss ein Team auf jeden Fall auch die Zeit für investieren, gelegentlich mal so einen Schritt zurückzutreten und aufs Ganze zu gucken. Ich glaube, das kann man auch verargumentieren und das ist auch notwendig, einfach um eine Software langfristig wartbar zu halten.
Anja Kammer: Wir haben jetzt schon sehr oft gesagt: Ja und wenn es genug Tests gibt oder wenn die Tests gut sind, dann kann man sich darauf verlassen, dass jetzt erst mal nichts Schlimmes schiefgeht. Ja, aber dann muss ich mich auf meine Tests verlassen können. Das ist auch ein wichtiger Punkt. Ich meine, unser Thema ist jetzt nicht Tests, aber wie stelle ich denn sicher, dass ich mich darauf verlassen kann, dass die Tests schon die schlimmsten Probleme finden werden, damit ich mich drauf verlassen kann, dass Code Reviews erst mal nicht so dringend sind?
Martin Otten: Ich glaube, ein Teil der Antwort ist wieder Code Reviews. Man sollte häufiger regelmäßig draufgucken, dort gemeinsam die Tests anschauen. Haben wir wirklich Akzeptanz-Tests? Wird die Fachlichkeit getestet? Wie sieht das aus? Natürlich spielen auch Metriken eine Rolle wie Testabdeckungen und so ein paar Kontrollen sollte ich entsprechend schon vielleicht auch automatisiert drin haben, dass der Code an sich getestet ist. Das bezieht sich meistens nur auf die Zeilen Code die getestet werden, jetzt nicht auf die logischen Branches. Und da glaube ich kein Weg an einem Code Review vorbei, um zu schauen wie die Testabdeckung aussieht. Auch da habe ich das Gefühl, dass Pull Requests da nur begrenzt helfen. Klar, kann ich immer gucken. Für diesen Change gibt es einen Test, aber insgesamt die Struktur der Tests zu bewerten: Wie sieht meine Testpyramide aus? Wie sicher sind wir eigentlich da unterwegs? Dafür brauche ich glaube ich noch mal ein gesondertes Review vom Code, um das regelmäßig zu bewerten.
Anja Kammer: Auf jeden Fall braucht es Richtlinien. Folgende Arten von Tests schreiben wir. Wenn du diese Arten von Änderungen machst, brauchen wir immer diese Art von Tests, beispielsweise. Ich beschäftige mich sehr viel mit Observerability und wenn es beispielsweise neue Logs gibt und wir wollen diese Logs verwenden, um Alerts zu produzieren, dann muss ich natürlich testen, dass diese Logs sich nicht ändern, weil ich natürlich mit Pattern Matching und Parsing darauf angewiesen bin, dass diese Logs sich in der Struktur nicht ändern. Und wenn sie sich ändern, dass ich dann auch eben diese Alerts anpasse, damit sie das richtige Parsing verwenden, damit die Alerts funktionieren. Das stimmt. Das heißt, es muss Richtlinien geben, vielleicht gibt es ein Template. Vielleicht gibt es ein Pull Request Template und man kann aber den eigenen Pull Request mergen, wenn man alle Häkchen abgehakt hat. Ist das eine Idee?
Martin Eigenbrodt: Ich bin so zwiegespalten mit Checklisten und Templates. Ich verstehe den Gedanken, weil es unheimlich viele Dinge gibt, auf die wir achten müssen. Testsabdeckungen gehören eben auch dazu, aber auch Security Themen gehören dazu. Das Design gehört dazu, es gehören ganz viele Sachen dazu und Checklisten haben die Tendenz sehr generisch zu sein. Was ich auf jeden Pull Request passen müssen und dann werden sie auch schnell mal abgehakt, weil man das Gefühl hat, das passt alles gar nicht. Das hat überhaupt nichts mit meiner Realität zu tun und da sehe ich immer die Gefahr. Ich glaube, Checklisten müssen, wenn man mit dem Team wachsen. Ich glaube, dass man die Kleinen einführen kann mit einem Punkt und dann wachsen lassen kann. Und wenn man eine Zehn-Punkte Checkliste oder ein Template am Anfang hat, dann führt es zu nichts, dass ein bisschen eine ähnliche Erfahrung, die ich mit statischer Code Analyse gemacht habe. So einen Tool loslaufen lasse und das spuckt mir 2000 Items aus für meine Codebasis. Dann wird sich genau gar nichts ändern. Wenn ich aber entscheide: Es gibt eine Regel, die ist jetzt besonders wichtig und wir etablieren die jetzt erst. Aber wenn wir die Erfahrung gemacht haben, wir haben Null Pointer Probleme oder Ressources Leaks und fokussieren uns auf das, dann ist es einfacher, das auch menschlich hinzukriegen, dass man sagt: Okay, wir wollen jetzt dieses Problem in Griff kriegen und das Tool hilft uns an der Stelle und macht uns nicht Vorschriften, sondern ist nützlich. Dass der menschliche Aspekt total wichtig ist, das Tool alleine ändert eben nichts, sondern erst mit dem Menschen zusammen.
Anja Kammer: Ja, genau. Als nächstes würde ich dann in das Thema gehen. Wie sollten PRs gestaltet sein, weil wir haben schon gerade darüber geredet. Es gibt ein paar Kommentare von den Kollegys.
Kollegy 10: Genau, wenn man Code Reviews macht, sollte man auf jeden Fall drauf achten, dass die möglichst klein sind und idealerweise immer nur eine Art von Änderungen beinhalten, damit man nicht gleichzeitig zwischen Refactoring und Feature Entwicklungen unterscheiden muss, sondern genau sieht: Wieso hat man jetzt diese Änderung gemacht?
Kollegy 11: Ich finde es wichtig, wenn man anfängt zu arbeiten, direkt ein Pull Request aufzumachen, den auch zu markieren mit ‚Work in progress‘, damit man als Team sieht, wer an was arbeitet und ob es Fortschritte gibt und das nach zu halten und man dann auch zwischendurch auch einfach mal Code Review machen kann.
Anja Kammer: Bei diesen Kommentaren haben darüber gesprochen, dass Requests möglichst klein sein sollten, möglichst früh aufgemacht werden sollten, um schon mal zwischendurch Reviews machen zu können. Anscheinend auch ungefragt. Ungefragte Code Reviews. Wie steht ihr dazu? Könnte das als persönlicher Angriff gewertet werden? Warum macht die Person jetzt auf einmal ein Code Review? Ich bin doch noch gar nicht fertig. Man vertraut mir nicht.
Martin Otten: Ich bin ein großer Freund davon. Dann ist es, wie die Kollegin gesagt hat, direkt transparent, woran alle arbeiten und es ist ein bisschen wie bei den Tests. Je früher ich etwas finde und Feedback geben kann, desto eher kann ich auch vermeiden, dass es am Ende größere Auswirkungen hat. Damit ich früher kommentieren kann und sagen dann: Hier, schau mal das, was du gerade machst. Bist du sicher, dass du es so machen möchtest? Dann steckt die Person nicht noch zwei, drei Tage da rein, das so weiterzumachen, sodass nachher die Änderungen zu korrigieren viel mehr Aufwand wird und vielleicht auch dann wieder schwieriger wird zu akzeptieren, dass man das dann noch mal umgestalten muss. Da ist man erstmal vielleicht schon wieder am Sprintende, sagt: Nein, jetzt bin ich an sich fertig. Kommt, lasst uns das nicht doch mergen. Und deswegen finde ich so früh wie möglich Feedback zu geben, das ist eine gute Sache.
Martin Eigenbrodt: Ich bin wieder ein bisschen zwiegespalten. Ich finde Transparenz gut und ich finde es gut, auf jeden Fall mindestens ein Branch zu pushen und meinetwegen auch einen ‚Work in Progress‘ aufzumachen, weil ich es furchtbar finde, wenn Menschen verschwinden für eine Woche mit ihrer Arbeit und man überhaupt nicht weiß, was passiert. Bei ungefragten Reviews bin ich ein bisschen skeptisch, was die Effizienz angeht, weil es so Phasen gibt, wo man vielleicht was ausprobiert oder die selbst sich noch nicht klar ist über eine Richtung. Und dann ist die Zeit vielleicht nicht richtig investiert an der Stelle.
Anja Kammer: Ja, das sehe ich genauso. Wenn ich bereit bin und was zum Zeigen habe, vielleicht habe ich nur ein ‚Work in Progress‘ Pull Request angelegt, weil ich erst einmal eine Idee implementieren möchte. Und wenn ich fertig bin, hätte ich eh nach Feedback gefragt und dann wäre es komisch ohne Kommentar meinerseits, dass jemand das reviewed und dann muss ich erst mal erklären: Ja, das war nur als Test, als Experiment gemeint. Und ich wollte dir auch sagen, warum ich das so gemacht habe. Und diesen Kontext hättest du gebraucht für dein Review. Ich mache es auch so, wenn ich Pull Requests erstelle und sie reviewen lassen möchte, dass ich auch selbst Kommentare für Dinge hinterlasse, wo ich ganz genau weiß. Hier wird es auf jeden Fall ein Kommentar geben, weil das sieht erst mal nicht richtig aus, oder das sieht merkwürdig aus. Dann gebe ich selbst ein Kommentar und sage: Bevor ich überhaupt den Kollegy Bescheid sage: Hey, mach mal ein Pull Request. Äh, nein, mach meinen Code Review. Ach übrigens, ich habe mir das und das dabei gedacht. Wundere dich nicht. Oder ich stelle selbst Fragen, wenn ich sage: Schau mal, ich habe das so gelöst. Meinst du, man kann das anders lösen? Oder wäre das und das hier viel besser? So benutze ich Pull Requests.
Martin Otten: Aber dann müsste ich eigentlich keinen Pull Request aufmachen.
Anja Kammer: Das stimmt.
Martin Otten: Wenn ich einen Pull Request aufmache, gehe ich davon aus, dass die Person offen ist für Feedback, weil es genau diese Funktionalität bereitstellt. Das kann Feedback geben und kann kommentieren und ich möchte es einfach auch anderen zeigen, sonst würde es reichen einen Branch zu pushen. Dann habe ich meine Änderungen safe im Repository und kann den Pull Request erst später aufmachen, wenn ich das Feedback noch gar nicht haben möchte.
Anja Kammer: Das stimmt. Für mich ist so ein Pull Request ein Quality of Life Tool. Es ist eine schöne Visualisierung. Ich weiß, wo es liegt und ich muss nicht erst wühlen in irgendwelchen Branches.
Martin Eigenbrodt: Ich habe noch was zu dem Einspieler, den wir gerade hatten. Und zwar war die Bemerkung, dass ich möglichst mehrere kleine Requests stellen will. Und da habe ich ein ganz praktisches Problem in dem Prozess viel in Teams. Und zwar arbeiten wir meistens mit Tool wie Jira zum Beispiel, wo unsere Storys, an dem wir arbeiten oder Tasks durch so ein Ticket repräsentiert sind, das wir von links nach rechts schieben. Und ganz häufig hängt der Review Prozess an diesem Ticket. Das heißt, ich bin fertig und schiebe mein Ticket in die nächste Spalte und jemand anderes nimmt sich das dann und reviewed es. Dann ist es aber zu spät für vorherige kleine Reviews und ich kann natürlich in meinem Git Repository Tool, was auch immer das ist vorher einen Merge Request erstellen, aber denen fehlt die Sichtbarkeit auf dem Board und ich habe da noch keine generelle Lösung gefunden. Häufig mache ich dann einen Link und tu ihn dann in den Chat und sage: Ich habe hier schon mal einen Merge Request. Das ist nicht fertig. Aber es wäre total gut, wenn das jetzt jemand anguckt. Und dann kommt sehr darauf an, wie die Last in dem Team gerade ist. Manchmal schnappt sich das dann jemand und man kriegt Feedback und manchmal folgen darauf 20 andere Nachrichten und dann ist es wieder weg. Und da wüsste ich gerne, ob ihr das Problem auch habt und wie ihr damit umgeht.
Anja Kammer: In unserem Team machen wir das genauso, dass wir den Chat verwenden und in der Chat Nachricht sagt man dann eben: Kann mal jemand schnell bitte? Dann weiß man: Ah, okay, ich sollte jetzt mal meine Arbeit beenden, weil diese Person wartet darauf. Ansonsten werden im Chat nach keinen Code Reviews gefragt, sondern das wird dann eben im Ticketsystem gemacht. Und am nächsten Tag, wenn man anfängt mit der Arbeit, macht man dann alle Code Reviews für den Tag und ist dann fertig mit Code Reviews. Mo, wie sieht es bei dir aus?
Martin Otten: Ich musste daran denken, dass es nicht nur Pull Requests klein sein sollten, sondern eigentlich auch Stories. Ich meine, weil wir eh kleine Storys haben, die wir schnell umsetzen können, die nicht tagelang oder wochenlang rumliegen. Und wenn ich das erreiche, dann habe ich das Problem natürlich auch nicht. Da habe ich eigentlich beides zusammen. Das ist natürlich schwierig, das geht nicht immer. Auch kleine Stories zu finden, ist Arbeit. So etwas aufzuteilen. Irgendwie muss man dann trotzdem einen praktikablen Weg finden. Eine Lösung, die mir einfällt, dass ich mit Sicherheit sagen kann: Ich habe einen Pull Request für die gesamte Story, da ist das gesamte Feature drin, das dann auch geschlossen live geht und ich kann trotzdem noch mal Sub-Tasks, die ich auch ein Jira anlegen kann. Da könnte ich noch mal separate Branches aufmachen, die dann auf diesen Feature Branch entsprechend als Pull Request reinlaufen. Dann könnte ich da schon mal Feedback zu bekommen und hätte das dann auch da strukturiert oder ich schaffe es, meine Änderungen so zu isolieren zumindest, dass ich sagen kann: Okay, die integriere ich auch wirklich früher. Was ich eigentlich noch schöner fände. Dann macht man natürlich mehrere Merge Requests für die einzelnen Sub-Tasks und hat dann keinen insgesamt mehr Merge Request für die Story am Ende. Und verlässt sich darauf, dass es dann am Ende alles zusammen funktioniert. Das geht natürlich auch nur mit guten Tests.
Martin Eigenbrodt: Ich wollte dir erst vehement widersprechen, aber du hast jetzt am Ende genau das gesagt, was ich sagen wollte. Ich bin ein großer Freund davon, Dinge so früh wie möglich zu integrieren, auch wenn sie vielleicht noch gar keinen Wert liefern. Wenn ich jetzt ein vorbereitetes Irgendwas tue, dann kann das schon in die Mainlinie, vielleicht nach dem Review eben. Und es gibt überhaupt keinen Grund, das irgendwo gammeln zu lassen. Da habe ich aber wieder das Problem, das abzubilden. selbst wenn die Story möglichst klein geschnitten ist, wenn ich das ideale Ziel erreicht hätte, kann das trotzdem dazu führen, dass ich verschiedene Merge Requests erzeugen muss. Und wenn ich die aber als Task oder Stories abbilde, dann verliere ich dieses Ideal, dass meine Tasks immer einen fachlichen Wert habe, das eben nur ein technischer Wert ist.
Martin Otten: Ich glaube, das sind zwei Sichten, die auch gar nicht zusammenpassen müssen. Wir haben einmal das, was wir machen in der Entwicklung, um diesen Wert zu schaffen, um diese Story umzusetzen. Und es gibt dann die Sicht der Story in Jira und ich glaube, das muss nicht deckungsgleich sein, dass es einen Merge Request und Pull Request gibt für diese Story. Und am Ende können wir die Story dann auf fertig schieben, wenn wirklich alles deployed ist und der Wert erreicht ist. Und das muss auch nicht für den Product Owner, für die Stakeholder sichtbar sein, sondern die bekommen dann natürlich auch erst Bescheid, wenn der Wert geliefert ist und sollten vielleicht dann nicht mehr einen Unterschied merken, welche Variante ich nehme.
Anja Kammer: Und dann haben sie auch keine Möglichkeit, dagegen zu argumentieren. Wenn man einfach die Arbeit macht, wie sie am besten gemacht werden sollte. Ja, ich bin auch der Meinung, dass man Refactorings gar nicht in Tickets packt, sondern einfach zwischendurch macht. Dann gibt es auch keine Diskussion, ob das jetzt sinnvoll ist. Vor allem natürlich mit anderen Stakeholdern als die Entwicklungsperson. So ein Entwicklungsteam weiß meistens, dass es ziemlich sinnvoll ist.
Martin Otten: Ja, und ich hatte auch schon Diskussionen, wer bezahlt denn jetzt das Refactoring?
Anja Kammer: Korrekt.
Martin Otten: Weil ihr habt das doch jetzt geliefert. Und wieso habt ihr das nicht direkt gemacht? Die Diskussion gibt es auch nicht, wenn man es entsprechend fortlaufend in die Arbeit integriert.
Anja Kammer: Genau. Ich hänge mich auch noch ein bisschen an der Aussage auf. „Ja, bei kleinen Änderungen brauche ich kein Pull Request, den kann ich direkt integrieren“. Aber was ist denn jetzt klein? Ist die Anzahl von Zeilen klein? Ist ein Pull Request klein, wenn ich eh nur Code lösche. Was ist klein? Ich kann mir jetzt vorstellen, eine Zeile Code kann auch inhaltlich riesig sein. Wenn es zum Beispiel ein RegEx ist und ich den komplett umgebaut habe, dann habe ich eine Zeile Code. Hoffentlich habe ich natürlich sehr viele Tests dann hintendran, aber dann ist das so gefühlt ein kleiner Pull Request. Aber vielleicht sollte ich das zumindest nicht als Regel festschreiben.
Martin Otten: Vielleicht kann man es ersetzen durch eine Änderung mit geringer Komplexität oder geringem Risiko. Wenn wir zum Beispiel in der IDE, die Refactoring-Funktion benutzen und Methoden und Klassen umbenennen habe ich eventuell hunderte und tausende Dateien, die angepackt wurden. Es gibt zum Beispiel auch in IntelliJ Autokorrektur Features, die man ziemlich safe benutzen kann, die stilistische Dinge anpassen. Da habe ich auch viele Dateien. Das denke ich, müsste trotzdem niemand reviewen, weil es ist einfach ein geringes Risiko, dass dabei was schiefgeht. Aber bei dem Beispiel mit dem RegEx, da bin ich natürlich auf jeden Fall ein Test anpassen, nur Tests erwarten. Aber vielleicht möchte ich dann doch eher noch mal ein Review haben.
Anja Kammer: Ja, genau, Es ist wieder eine Kontextentscheidung und eine inhaltliche Entscheidung. Also Risiko. Wie du sagtest, Impact und Risiko sollte der entscheidende Faktor sein, ob es nun einen Pull Request und dementsprechend einen Code Review geben sollte oder nicht. Das nächste Thema ist Wissen verteilen und dazu haben wir schon gesprochen. Aber vielleicht wollen wir noch tiefer reingehen.
Kollegy 12: Code Reviews finde ich immer eine super Gelegenheit, um Wissen im Team zu verteilen. Ich lerne super viel von meinen Kollegen und kann dabei eben auch meine Erfahrungen dann weitergeben.
Anja Kammer: Ja, ich hatte ja schon gesagt, auch für mich ist das ein wichtiger Punkt. Ich habe durch Pull Requests gelernt, überhaupt gut zu programmieren und einen guten Stil entwickelt. Und ich finde auch, dass ich unfassbar viel lerne dadurch und aber auch wenn ich später selbst ein Feature einbauen soll, kenne ich den Code gleich viel besser, weil ich den schon mal gereviewed habe. Und wer mich kennt, der weiß: Meine Reviews sind sehr tiefgreifend und umfassend. Das heißt, ich kenne mich dann wirklich in dem Code aus und habe auch das Gefühl, Verantwortung dafür übernehmen zu können. Kommen wir wieder mit dem Verantwortungsgefühl.
Martin Eigenbrodt: Ja, auf jeden Fall. Wir haben das schon mehrfach gesagt, der Hauptzweck ist bei so einem Review das Wissen verteilen, und zwar in beide Richtungen. Einmal durch Feedback, andererseits durch Zeigen: Guck mal, was ich hier an Struktur neu geschaffen habe oder an Features oder an Dingen. Und das ist total wichtig, weil in dem Rahmen von so einem Review häufig dann auch Kommunikation stattfindet, entweder im Chat oder manchmal auch im Gespräch. Das ist übrigens etwas, was ich auch oft mache, wenn ich das Gefühl habe, ich kriege das jetzt nicht ordentlich aufgeschrieben, was ich kommentieren möchte, dann rufe ich eben an und dann sprechen wir über die Änderungen. Und das ist ein guter Zeitpunkt so ein Code Review um das Wissen zu verteilen im Team. Und geschriebene Dokumentation wird es eben nicht ersetzen können, sondern nur ergänzen. Du hast gerade gesagt, dass du sehr tief reviewst und dann auch viel Wissen hast an der Stelle. Das ist tatsächlich ein Punkt, wo ich mich schwertue, wegen diesen Kontextwechsel. Wenn ich ein Code Review wirklich tief machen soll, bedeutet das, dass ich möglicherweise einen komplett neuen Kontext mir erschließen muss und ich fände das gut, das zu tun. Aber da muss ich ehrlich sagen, das gelingt mir nicht immer, das so zu schaffen, wenn es eben zu viele verschiedene Kontexte gibt in so einem Projekt oder Team.
Anja Kammer: Das stimmt auf jeden Fall. Und ich glaube, es ist auch ein sehr schwieriges Thema, dass die Anzahl der Domänen oder der Bereiche, die man bearbeiten muss, als Team möglichst klein gehalten werden muss, damit auch jedes Teammitglied sich mit dem Code auskennen kann. Und ich glaube, wenn man zu dem Punkt kommt, dass man Arbeit abgibt, weil man sagt, ich kenne mich mit dem Service nicht aus, sollte man dann nach Hilfe schreien und sagen: Wir brauchen eine Restrukturierung, wir brauchen eine Separierung, wir können uns nicht mehr mit allen Domänen beschäftigen, weil uns der Kopf platzt.
Martin Otten: Ich fände es auch schwierig, wenn Pull Requests die einzige Möglichkeit wären, Wissen zu tauschen oder das Hauptwerkzeug, um Wissen zu tauschen im Team. Es gibt schon mal die Situation, dass die Person, die gerade an einem Change auch arbeitet, krank wird, dass sie im Urlaub ist, ausfällt. Und wenn dann wirklich jede Person im Team lange brauchen würde, sich da einzuarbeiten, das Review zu machen und diese Aufgabe zu übernehmen, dann habe ich eventuell ein Problem und auf jeden Fall ein hohes Risiko im Team. Das mir genau das auf die Füße fallen kann. Das ist natürlich ein wichtiger Aspekt von Pull Requests Wissen zu verteilen. Ich sollte aber auch so sicherstellen, dass möglichst viele im Team sich mit dem Code auskennen, wissen, welche Änderungen anstehen, wie sie umgesetzt werden und dann zum Beispiel auch übernehmen könnten, wenn entsprechend zum Beispiel eine Person ausfällt.
Anja Kammer: Unabhängig, ob man Code Reviews macht oder nicht. Das sollte nicht das einzige Mittel sein, um sich auszukennen. Verstehe. Ja, stimmt. Gut, das nächste Thema wäre der soziale Aspekt. Und das ist auch unser letztes Thema, um noch ein bisschen Fröhlichkeit in diesen Podcast reinzubringen. Denn bei unserem Firmen Event gab es ganz nette Kommentare dazu.
Kollegy 13: Good Reviews sind wichtig, sollte man machen, weil sie die Interaktion fördern zwischen Entwicklis.
Kollegy 14: Ich bin ein absoluter Freund von Code Reviews. Wenn mein Code auch reviewed wird. Ich finde es besonders schön, wenn ich Rückfragen dazu bekomme, die mir wertvollen Input geben. Dadurch lerne ich was. Und was ich besonders schön finde ist, und ich hatte das mal in einem Projekt, dass auch positive Dinge einfach mit einem Emoji kommentiert werden, weil es einfach oft vergessen wird, dass man auch positive Dinge oder Sachen, die man schön programmiert hat, einfach auch wahrgenommen werden.
Kollegy 15: Die letzte Nachricht in einem Pull Request sollte immer ein Party GIF enthalten. Weil das schön ist und man sich darüber freut.
Anja Kammer: Ja, stimmt. Ich freue mich auch immer darüber. Und wenn es auch nur diese Emoji Reaktion ist, wenn ich ein Approval für mein Pull Request bekomme, dann einfach so eine Rakete oder ein Eichhörnchen oder so. Ich freue mich dann immer riesig. Vielleicht auch unterbewusst.
Martin Otten: Ich finde das auch unheimlich wichtig. Ich habe es am Anfang auch schon erwähnt, dass man natürlich auch positive Dinge erwähnen sollte, weil sonst Code Reviews, Pull Requests dann doch sehr negativ sein können, wenn man immer nur das reinschreibt, was gerade nicht so toll war. Und natürlich gibt es auch oft positive Dinge zu erwähnen und das sollte man auch tun.
Martin Eigenbrodt: Ja, ich finde es witzig. Ich merke, dass ich eher der spröde Typ bin bei sowas. Ich nehme für mich mal mit, mehr Party GIFs zu verwenden. Schaden kann es ja nicht. Und tatsächlich erlebe ich es auch, dass ich das positiv finde, wenn sowas kommt. Es ist auf jeden Fall ein interessanter Aspekt.
Martin Otten: Die erste Stimme, die sprach auch von Code Reviews im Allgemeinen. Und ich persönlich bin der Meinung, dass generell eine andere Form von Code Reviews noch mehr Team-Interaktion fordern können. Wenn ich mich zusammensetze in einem virtuellen oder im echten Meeting-Raum, eine Person oder ein paar präsentieren, was sie gemacht haben, zeigen den Code. Man spricht ihn durch. Vielleicht bestellt man sich dann noch eine Pizza dazu. Wenn man sich vor Ort trifft, dann kann das viel Interaktion fördern, wohingegen ein Pull Request an sich, je nachdem auch sehr kurz und knapp sein kann. Und sehr funktional und vielleicht gar nicht so viel Kommunikation fördert. Das ist sicherlich je nach Team unterschiedlich.
Martin Eigenbrodt: Puh, ich glaube, das stellt ganz schön hohe Anforderungen an dein Team. Wenn ich mir diese Situation vorstelle, ich vielleicht gerade irgendwie als Junior in so einem Projekt bin und dann gibt es eine Meetingraum-Präsentation mit fünf Leuten. Und am besten ist, wenn man also Strukturen hat, noch so ein Team Lead dabei und dann wird mein Code allen gezeigt und darüber gesprochen. Ich glaube, heute könnte ich das ab. Aber es gibt Zeiten, wo ich das furchtbar gefunden hätte und da muss man viel Fingerspitzengefühl haben. Das ist das Schöne an diesem Merge Request, dass das, obwohl sie natürlich faktisch sichtbar sind für alle, fühlen die sich aber geschützter an, weil es meistens ein Dialog zwischen zwei Personen ist. Und da ist es ein bisschen einfacher, mit so Feedback umzugehen. Und dieser Aspekt, dass man sich so bloßgestellt fühlt, ist da schwächer auf jeden Fall in meiner Wahrnehmung.
Anja Kammer: Ich kenne das auch aus der Praxis, dass es so etwas wie ein Backend Club gab. Es gab auch ein Frontend Club, aber das wurde getrennt, weil die Skills nun mal so verteilt waren in diesem Projekt. Und in diesem Backend Club, wo ich dann dementsprechend teilgenommen habe, wurde eben dann Code rausgezogen, der so ungünstig war und das muss man abkönnen. Wenn es der eigene Code ist und dann haben alle über diesen Code gesprochen und ihn auseinandergenommen. Aber dann wurden auch Spiele gespielt. Ja, schreib den Code doch noch mal um und dann schauen wir mal, was dabei rausgekommen ist bei euch und dann bewerten wir von allen den Code. Wie hättet ihr es geschrieben? Vielleicht kann man das auch ein bisschen spielerisch machen.
Martin Otten: Das, was du beschreibst, klingt aber auch so, als ob das auch Vertrauen schaffen kann. Klar, ist das eine herausfordernde Situation, aber wenn man wirklich Feedback dann auf Augenhöhe gibt und wenn man zwar kritisch dem Code gegenüber ist, aber nicht der Person und dann auch noch so ein Spiel spielt, wie du das gerade beschrieben hast, kann ich mir auch vorstellen, dass das wirklich zum Zusammenhalt des Teams beiträgt und zum Vertrauen.
Anja Kammer: Ja, und wie der Kollege gesagt hat: Teaminteraktion fördern. Und das wäre genau so eine Maßnahme. Gut, was haben wir also mitgenommen aus unserem Gespräch? Ich habe mitgenommen, diesmal auch bewusst mehr Emojis in Pull Requests fühlen sich alle irgendwie gut. Was habt ihr mitgenommen?
Martin Eigenbrodt: Für mich ist noch mal klar geworden, wie wenig isoliert das Thema ist, sondern wie viel Code Reviews zu tun hat mit der Struktur des Teams, mit Verantwortungsgefühl, mit Vertrauen untereinander. Aber wir haben auch den Aspekt berührt, dass es zu tun hat damit: Wie sind eigentlich Stories geschnitten? Wie ist meine Organisation? Wie ist das Vertrauen von außen, was jetzt das Maß an Refactoring zum Beispiel angeht. Da sieht man, wie es sehr stark in einen riesigen Kosmos eingebettet ist. Und man muss das alles im Blick behalten. Und deshalb gibt es, glaube ich auch nicht so eine Ja-Nein Antwort, will ich immer Code Reviews oder nicht? Oder mache ich jetzt immer hier Show, Ship, Ask oder so, sondern es ist wirklich immer so ein hin und her mit all den Nebenbedingungen, die wir da haben.
Martin Otten: Ich würde mich dem eigentlich anschließen. Ich fand es auch spannend, wie viele verschiedene Aspekte wir betrachtet haben und ich habe jetzt trotzdem auch das Gefühl, dass wir eigentlich auch zum C3 jetzt gar nicht so weit auseinander liegen, was das Thema angeht, auch wenn wir da unterschiedliche Präferenzen haben.
Anja Kammer: Ja, das Wichtigste ist, darüber zu reden im Team. Und wenn man darüber redet und es klar macht und explizit macht, was man sich wünscht, dann kann die Zusammenarbeit auch gut funktionieren. Ja, vielen Dank, dass ich mit euch sprechen durfte. Vielen Dank Martin Eigenbrodt.
Martin Eigenbrodt: Ja, gerne.
Anja Kammer: Und vielen Dank, Martin Otten.
Martin Otten: Es war mir eine Freude.
Anja Kammer: Dann bis zum nächsten Mal. Tschüss.
Martin Eigenbrodt: Tschüss.
Martin Otten: Tschüss.