Engineering Kiosk Episode #16 Code Reviews: Nützlich oder bremsen nur ein gutes Team?

#16 Code Reviews: Nützlich oder bremsen nur ein gutes Team?

Diese Episode in deiner Podcast-App hören...

Shownotes / Worum geht's?

Code Reviews: Jeder will schnelles Feedback, doch niemand hat Zeit dafür - Eine Hassliebe.

Eine Komponente im Alltag jedes Software Engineers. Egal ob Junior, Senior oder Staff-Engineer. Jeder erstellt Code Reviews und kommentiert die Arbeit von den Kollegen. Doch wie sehen gute Code Reviews aus? Was gehört hinein, was bleibt besser draußen? Wie viel Reviewer machen Sinn? Wie geht man mit Nitpicking-Kommentaren und Gatekeepern um? Und allgemein: Zieht dieser zusätzliche Schritt nicht die Performance des Teams runter und ist sowieso Overhead?

All das und noch viel mehr in dieser Episode zum Thema Code Reviews.

Bonus: Was Faultiere und Markus Söder mit Code Reviews zu tun haben und warum Blubberwasser den Charakter verdirbt.

Feedback an stehtisch@engineeringkiosk.dev oder via Twitter an https://twitter.com/EngKiosk

Sprungmarken

(00:00) Intro

(01:23) Amsterdam, Hipster und Craft-Bier

(02:04) git rebase, history rewrite, force push und Kollaboration

(04:54) Saubere git history und Merge-Commits

(05:17) Code Reviews

(02:05) Was ist denn ein Code Review?

(07:02) Muss der Code Reviewer immer mehr Erfahrung haben?

(08:10) Was wir von Juniors im Job und bei Code Reviews lernen können

(09:37) Was ist denn ein Google Fellow?

(10:37) Code Reviews als Mittel fürs Onboarding

(11:35) Frühes pushen der Arbeit ist wichtig für frühes Feedback

(13:15) Wo sollen die Informationen zu meiner Änderung stehen? Im Pull Request oder in der Git Commit message?

(15:04) Wen lade ich zu meinem Code Review ein? Code Owners file und Software Repository Mining

(17:34) Worauf soll ich als Code Reviewer acht geben und konzentrieren?

(20:09) Pro-Aktivität in Code Reviews mit aktiven Vorschlägen und Fragen stellen

(23:33) Muss man jedes Feedback aus dem Code Review annehmen? Lass das Ego vor der Tür und Gate-Keeper

(26:37) Kommunikationskonflikte in Code Reviews: Video Calls vs. geschriebene Kommunikation

(28:14) Nitpicking, Code-Formatting, Styleguides, Linter und Automatisierung

(32:11) Mindern Pull Requests und Code Reviews die Performance von Teams?

(36:56) Schnelligkeit bei Facebook und Booking: Schnelles Mergen und Code Reviews danach

(39:01) Auslagerung der Verantwortung bei Code Reviews und Blameless Kultur innerhalb der Firma

(42:38) Technische Mittel für sichere Deploys: Feature Flags, Canary Deployments, Dark Launches und Blue-Green-Deployments

(46:08) Wie schnell sollte ein Code Review vom Team gereviewed werden?

(53:00) Code Reviews in der Firma vs. Code Reviews in Open Source und die Motivation

(54:34) Konsequenzen bei nicht eingehaltenen Service Level Agreements (SLA)

(57:28) Outro

Hosts

Engineering Kiosk Podcast: Anfragen an stehtisch@engineeringkiosk.dev oder via Twitter an https://twitter.com/EngKiosk

 

Transkript

Das Transkript wurde automatisiert per Speech-to-Text erstellt und kann daher Fehler enthalten.

Andy Grunwald (00:00:04 - 00:00:49) Teilen

Eine neue Woche bedeutet auch eine neue Episode vom Engineering Kiosk. Heute dreht sich alles um Code Reviews, eine Hassliebe. Jeder will schnelles Feedback, doch niemand hat Zeit dafür den Code von anderen durchzugucken. Dennoch ist es eine wichtige Komponente im Alltag jedes Software Engineers, egal ob Junior, Senior oder Staff Engineer. Doch wie sehen eigentlich gute Code Reviews aus? Was gehört hinein? Was bleibt besser draußen? Wie viele Reviewer machen Sinn? Wie geht man mit Nitpicking, Kommentaren und Gatekeeper um? Und allgemein, zieht dieser zusätzliche Schritt nicht die Performance des Teams runter und ist sowieso Overhead? Diese und andere spannende Fragen, wie zum Beispiel, was hat Markus Söder, Faultiere, Service Level Agreements, Stullen und Kniffen mit Code Reviews zu tun? All das klären wir in dieser Episode.

Wolfi Gassler (00:00:53 - 00:01:05) Teilen

Man hört eigentlich nur Andis Geploppe von seinem Bier. Und ich hab mir ja gedacht, er geht heute wieder sporteln, damit wir mal Bier trinken können. Aber heute ist seine Ausrede, wir müssen Bier trinken, weil er so einen argen Muskelkater hat von gestern der Arme.

Andy Grunwald (00:01:05 - 00:01:22) Teilen

Man muss aber auch dazu sagen, der Wolfgang ist spät zur Podcastaufnahme, weil er noch schnell laufen musste. Das bedeutet, er ging, also das nenne ich mal Einsatz, er geht durch die Innenstadt von Amsterdam eine Runde laufen, damit er abends mit uns ein Bier trinken kann. Ich finde das schön.

Wolfi Gassler (00:01:22 - 00:01:32) Teilen

Genau, und ich habe heute sogar richtig Amsterdamer Bier. Ein Eiwitt ist ein Weißbier aus Amsterdam. Ist kein klassisch deutsches Weißbier, wie man es so kennt, aber es ist ganz gut.

Andy Grunwald (00:01:33 - 00:01:36) Teilen

Ist das in Amsterdam nicht eher so, dass die ganzen Hipster irgendwie immer so Craft-Bier trinken?

Wolfi Gassler (00:01:36 - 00:01:41) Teilen

Ja, das ist eines der lokalen Brauereien, genau so Craft-Bier. Also Prost.

Andy Grunwald (00:01:41 - 00:01:41) Teilen

Prost.

Wolfi Gassler (00:01:43 - 00:02:09) Teilen

Ihr wurde auch in meinem Haushalt gechallenged zur letzten Episode 15, wo wir über Comments gesprochen haben. Und ihr habt heute schon die Frage bekommen, dieses Git-Rebase. Ist es nicht eigentlich schlecht, wenn ihr Git-Rebase macht und auf dem Branch busche, weil dann die History gerewritet wird und wenn ihr dann mit anderen zusammenarbeitet auf einem Branch, dann hattet ihr Probleme durch den History-Rewrite? Kannst du mir das beantworten?

Andy Grunwald (00:02:10 - 00:03:01) Teilen

Die klassische Antwort wäre, it depends. Weil es kann schlimm sein, es kann aber auch nicht schlimm sein. Um auf deine Frage zurückzukommen, ist das schlimm, ein Git-Rebase? Nein, ein Git-Rebase selbst ist erst mal nicht schlimm. Die Frage ist eigentlich nur, wie viele Leute haben diesen Branch, den du da bereits, den du rebasen möchtest, denn auf ihrer Platte schon. Und arbeiten die da Leute drauf? Weil wenn du ein Git-Rebase machst, Dann nimmst du eigentlich deine Changes, packst sie zur Seite, holst dir alle aktuellen Commits und packst deine Changes wieder oben drauf. Wenn du das natürlich pushst ins Repository, was als Hauptrepository angesehen wird, dann haben die anderen, die den Branch ebenfalls ausgecheckt haben, natürlich ein Problem, weil deren Lokalhistory weicht von der auf dem Server ab.

Wolfi Gassler (00:03:02 - 00:03:52) Teilen

Man merkt schon, dass du dich gut auskennst bei Git. Ich bin natürlich nicht so gut ausgekannt und habe mir gedacht, ist das wirklich ein Problem? Ich hatte eine Vermutung, habe dann gleich meinen Entwickler meines Vertrauens probiert anzuschreiben, den Tim, der mir damals das Rebase eingeredet hat, vor Jahrzehnten schon fast, der war natürlich nicht erreichbar und hat nicht innerhalb drei Minuten geantwortet. Also habe ich das ganze selber ausprobiert, in einem kleinen Versuch. Und du hast vollkommen recht, sobald du rebased, musst du die Git-History rewriten und damit einen forced push machen. Und wenn jemand anderer auf demselben Branch arbeitet und eine alte Version hat, dann muss er einen Pull rebase machen, damit er überhaupt einen Pull machen kann. Ein klassisches Pull funktioniert gar nicht, weil eben eine neue History vorhanden ist. Also, wenn man rebased Branches pusht, kann man in Probleme laufen, ja.

Andy Grunwald (00:03:52 - 00:04:18) Teilen

Ich nutze rebase jetzt nicht so oft, aber ich stehe auch nicht unbedingt auf eine unglaublich saubere Git-Historie. In einem Pull-Request zum Beispiel finde ich das völlig okay, wenn der Pull-Request einfach mal einen Monat oder zwei offen ist und man die ersten paar Kommits gemacht hat und dann holt man sich den Master-Branch, den Merge man wieder in seinen Entwicklungs-Branch rein. Dann hat man natürlich mehrere Commits da, ja. Einmal seine eigenen und dann die vom Hauptstrang.

Wolfi Gassler (00:04:18 - 00:04:22) Teilen

Aber du würdest den reinmerchen, nicht reinrebasen?

Andy Grunwald (00:04:22 - 00:04:26) Teilen

Ich merge den immer rein. Auch bei meiner ganzen Open-Source-Arbeit. Ich merge den immer rein.

Wolfi Gassler (00:04:27 - 00:04:53) Teilen

Damit hast du natürlich das Problem nicht mehr, dass du nicht pushen kannst oder die Probleme, die durch das Pushen entstehen. Wäre ganz interessant, wie das sonst so Entwickler und Entwicklerinnen machen da draußen. Ihr könnt uns gerne schreiben, wie ihr das so handhabt. Wäre wirklich interessant, wie das Teams dann die, die Rebase verwenden, auch wirklich handhaben, ob man dann einfach erst am Ende ein Rebase macht oder ob die sehr kurzlebige Branches haben. Ich glaube, da gibt es mehrere Herangehensweisen.

Andy Grunwald (00:04:54 - 00:05:06) Teilen

Ich weiß auf jeden Fall von ein paar Firmen, die sagen ganz, das ist deren Rule-Set, die sagen, okay, bevor ein Branch gemerged wird, wird der einmal gerebased komplett und dann geht's weiter.

Wolfi Gassler (00:05:06 - 00:05:08) Teilen

Also erst am Ende, bevor man merged.

Andy Grunwald (00:05:08 - 00:05:08) Teilen

Ja.

Wolfi Gassler (00:05:09 - 00:05:47) Teilen

Dann haben wir das auch geklärt. Dann sind alle Personen in meinem Haushalt wieder zufrieden und können schlafen. Sehr gut. Aber dann kommen wir mal auf das eigentliche Thema in dieser Episode. Wir wollten es ja eigentlich letztes Mal schon machen, weil es sehr stark zusammenhängt eigentlich mit Kommentieren und Dokumentieren. Aber uns ist einfach die Zeit ausgegangen. Darum besprechen wir es in dieser Episode. Und zwar geht es um Code Reviews. Anetu bist ja ein Meister der Zusammenarbeit mit den Teams und hast schon mit hunderten Teams zusammengearbeitet, mindestens. Hast damit wahrscheinlich auch viele Code Reviews hinter dir. Was ist denn ein Code Review? Wie würdest du ein Code Review beschreiben?

Andy Grunwald (00:05:47 - 00:06:41) Teilen

Ich würde sagen, Code Review ist erstmal eine Aktivität, wo ein Programmierer oder ein Software-Entwickler, ein Software-Engineer, den Code von einem anderen oder mehreren anderen Software-Engineers reviewed, drüber schaut, sind da grobe Schnittzeilen drin. Um einfach ein 4-Augen-Prinzip, 4, 6, 8-Augen-Prinzip zu haben, weil ein einzelner Software-Ingenieur kann natürlich tagelang an einer Änderung arbeiten und man kann da irgendwie ein bisschen arbeitsblind werden. Und besonders in nicht typisierten Sprachen und Skriptsprachen kommt es auch sehr, sehr oft mal vor, dass man zum Beispiel einen Tippfehler in variablen Namen hat und so natürlich dann irgendeinen Fehler einschleicht. Summa summarum schaut aber mindestens ein anderer Programmierer über die Arbeit von einem anderen drüber, um einfach eine Art von Grundqualität sicherzustellen.

Wolfi Gassler (00:06:41 - 00:06:54) Teilen

Bedeutet das für dich, dass das immer irgendwie dann ein senioriger Developer machen muss, weil der irgendwie mehr Ahnung hat und wenn du von drüber schauen redest, dann, dass das irgendwie so eine Qualitätskontrolle ist?

Andy Grunwald (00:06:54 - 00:08:24) Teilen

Für mich bedeutet das erstmal, dass jeder darüber schauen kann, unabhängig von der eigenen Erfahrung. Ich habe zum Beispiel ganz tolle Code Reviews gekriegt von Leuten, die frisch der Firma gejoined sind, die dann auch Junior Software Engineer waren. Und die haben ganz klassische Fragen gestellt, warum machst du das denn hier so und warum nicht so wie in diesem Medium Artikel, haben sie in irgendeinem Artikel verlinkt. Teilweise hat mich das auch echt zum Nachdenken gebracht, warum ich das denn wirklich so mache. Und das bringt mich natürlich auch in eine Situation, wo ich dann natürlich meine Designentscheidungen oder meine Entscheidungen, die ich während des Programmierens getroffen habe, irgendwie erklären möchte. Auf der einen Seite erklären, auf der anderen Seite ist das aber auch so eine Art, vielleicht sogar schon Mentoring-Situation, dass man jemanden ein bisschen Wissen teilt. Von daher, das kann erstmal jeder reviewen. Natürlich, umso mehr Erfahrung die Person hat innerhalb der Firma und auch mit der Komponente, mit der Applikation, die man anfasst, umso wertvoller kann natürlich der Review im Kontext der eigentlichen Änderung sein. Aber Erfahrung ist hilfreich, aber dass jemand drüber schaut, mein grober Schnitzer zum Beispiel, sollte eigentlich auch Sollten auch neue Dinge sehen. Hast du schon mal eine Situation gehabt, wo ein Junior Engineer deine Arbeit als erfahrener Doktor mal gereviewt hat und vielleicht sogar dir hilfreiche Tipps gegeben hat, um da ein besseres Ergebnis rauszukriegen?

Wolfi Gassler (00:08:24 - 00:09:38) Teilen

Natürlich nicht. Ich bin unfehlbar. Ist ja ganz klar. Natürlich und ich sage das ja auch immer wieder, weil man so oft sagt, neue Mitarbeiter oder Mitarbeiterinnen am Anfang, die ersten Monate sind wertlos für die Firma, bis jemand wirklich im Job ist. Ich bin da eigentlich komplett anderer Meinung. Natürlich ist jemand nicht so schnell und kann nicht so schnell Features schreiben und an den Start bringen. Aber es hat jeder diesen Status von einem Neuling und kann extrem dumme Fragen stellen und einfach mal fragen, genau wie du es gesagt hast, warum macht ihr das so, warum machst du das so, könnte man das auch so machen? Und je dümmer die Fragen, umso besser das Ergebnis. Und ich habe das auch immer Studenten gesagt, wenn sie angefangen haben, dass die Also nicht an der Uni, sondern in Jobs. Deren Fragen können so hilfreich sein und ein ganzes Team komplett auf einen anderen Weg führen, weil plötzlich das Team versteht, dass sie irgendein Bullshit gebaut haben, aber es war einfach so Standard in diesem Team, dass man es immer so gemacht hat und dann kommt irgendeine neue Studentin ins Team und fragt, hä, warum macht ihr das so? Und plötzlich kann niemand eine Antwort geben und man kommt drauf, dass das vielleicht ein komplett falscher Approach war. Also auch das habe ich schon öfters miterlebt und das ist extrem wertvoll und kann natürlich dann auch in Code Reviews dementsprechend auch so wertvoll sein.

Andy Grunwald (00:09:39 - 00:09:51) Teilen

Aber ich meine im Endeffekt ist es ja auch total egal ob du Junior Software Engineer bist, Mid-Level Senior Staff oder Principal oder sogar Distinguished oder Google Fellow ja also ist doch total egal was du für ein Titel.

Wolfi Gassler (00:09:51 - 00:10:01) Teilen

Was ist denn Google Fellow du kommst immer mit neuen Titeln dran jetzt hast du mir dieses Distinguished schon mal erklärt das war schon neu für mich jetzt kommst du mit Google Fellow was sind diese Kollegen?

Andy Grunwald (00:10:02 - 00:10:26) Teilen

Google fellow ist der höchste engineering titel in der software engineering career letter von google selbst und das ist man dann höher als cto. Ich glaube ganz kurz davor gefühlt es gibt so viel ich weiß auch nur zwei leute die den status google fellow haben und das sind die beiden leute die unter anderem ich glaube big query gebaut haben google big query. Verlinken wir gerne in den Shownotes.

Wolfi Gassler (00:10:26 - 00:10:28) Teilen

Also die absoluten Gurus.

Andy Grunwald (00:10:28 - 00:10:38) Teilen

Ich sag mal so, ich würd schon gern mal mit denen eine Runde arbeiten. Ich würd schon gern mal wissen, wie die so arbeiten, wie die so drauf sind bei komplexen Architekturen.

Wolfi Gassler (00:10:38 - 00:10:40) Teilen

Also einen Code-Review von denen bekommen.

Andy Grunwald (00:10:40 - 00:10:51) Teilen

Ich würd auch gern mal einen Code-Review von denen bekommen, weil ich denke, durch einen Code-Review erfährt man auch eine ganze Menge über einen Menschen. Also wie die Person kommuniziert und so weiter und so fort.

Wolfi Gassler (00:10:52 - 00:10:55) Teilen

Jetzt kommen wir schon wieder auf eine schöne Meta-Ebene.

Andy Grunwald (00:10:55 - 00:11:38) Teilen

Aber was ich gerade noch sagen wollte, unabhängig von dem Titel, mit dem man in einer Firma einsteigt, sind Code Reviews eigentlich eine super Sache fürs Onboarding, weil ich denke durch Code Reviews kriegt man ein gutes Gefühl, welche aktuellen Probleme in der Firma gerade behoben werden, wie diese behoben werden, wie groß die Changesets sind, die Reviewkultur, die Kommunikationskultur etc. Man kriegt ein sehr gutes Gefühl, nur wenn man Code Reviews beobachtet und auch mal mitliest. Kann ich neuen Leuten nur empfehlen, die bei einer Firma anfangen. Nehmt euch mal einen Tag, zwei, drei oder vielleicht einfach nur mal eine Stunde pro Tag in den ersten paar Wochen und lest euch mal die aktuellen Pull Requests durch.

Wolfi Gassler (00:11:38 - 00:11:48) Teilen

Was soll denn jetzt in so einem Code Review drinstehen? Was würdest du sagen, auf was soll man achten? Was soll man vielleicht nicht reinschreiben? Was soll man reinschreiben? Wo soll der Fokus liegen?

Andy Grunwald (00:11:48 - 00:11:50) Teilen

Als Autor oder als Reviewer?

Wolfi Gassler (00:11:50 - 00:11:58) Teilen

Als Reviewer. Oder auch als Autor. Solltest du als Autor irgendetwas Spezielles beachten, wenn du ein Code Review anforderst?

Andy Grunwald (00:11:58 - 00:13:19) Teilen

Dann starten wir mal am Anfang. Du machst einen Branch auf, machst deine Änderungen, bis zum Programmieren, Hack, Hack, Hack, buff, hast ein super Ergebnis. Push die ganze Sache zu deinem Git-Server. Nehmen wir mal jetzt GitHub als Beispiel, das ist gerade recht einfach. Meine erste Frage ist, wann pushst du denn diesen Branch? Pushst du ihn, wenn du mit all deinen Änderungen fertig bist? Oder pushst du ihn sofort nach der Anlage des Branches? Oder pushst du ihn auch noch während der Entwicklungszeit? Meines Erachtens nach solltest du deinen Branch so früh wie möglich an den zentralen Git-Server pushen, auch wenn deine Änderungen noch nicht fertig sind. Warum? Erstens, es schützt dich vor Datenverlust, falls deine Festplatte gerade mal hoch geht oder dein Laptop abbrennt oder was weiß ich, dann hast du deine Änderungen auf jeden Fall noch auf dem zentralen Git-Server. Zweitens, Du kannst in den meisten Systemen, und jetzt bleiben wir mal wieder bei GitHub, einen Draft Pull Request eröffnen. Das bedeutet, hey, es kommt ein Change, dieser ist aber noch Work in Progress, deswegen ist er noch nicht offen für Code Reviews. Leute haben aber trotzdem die Möglichkeit, sich den Change schon mal anzusehen, schon mal eine Idee zu kriegen, was machst du denn da. Und das ist auch so eine Art nennen wir es mal Frühwarnsystem, zu sagen, hey, ich arbeite gerade an was. Und so kann auch jeder deinen Progress ein bisschen tracken, ohne dass du eigentlich was machst. Das ist eine tolle Sache. Deswegen bin ich der Freund davon, push dein Branch so früh wie möglich.

Wolfi Gassler (00:13:20 - 00:13:30) Teilen

Wenn du jetzt deine Branchentwicklung abgeschlossen hast und in die nächste Phase kommst, Code Review anforderst, wie machst du das? Was machst du da? Wie wählst du die Leute aus, die ein Code Review machen sollen?

Andy Grunwald (00:13:31 - 00:13:58) Teilen

Als allererstes mache ich mal einen ordentlichen Pull-Request. Ein ordentlicher Pull-Request bedeutet ordentlicher Titel, ordentlicher Text. In dem Text sollte drinstehen, welches Problem dieser Pull-Request löst, wie man das testen kann, vielleicht eine kleine Historie, was man denn wirklich so gemacht hat. Ich habe die Docs geupdatet, ich habe noch drei Unit-Teste zugefügt und vielleicht nochmal einen Link zu einem Ticket, wo gegebenenfalls mehr Informationen stehen.

Wolfi Gassler (00:13:58 - 00:14:27) Teilen

Das ist jetzt sehr schön. Das haben wir in der letzten Episode, Episode 15, wer sie noch nicht gehört hat, zu den ganzen Comments auch die Git-Commit-Messages und so weiter besprochen, weil der Andi gemeint hat, er schreibt viel zu kurze Git-Commit-Messages und jetzt erklärt er, was er nicht alles in den Bull-Request reinschreibt. Also wenn man schon die schönen Commit-Messages macht, dann hat man im Bull-Request eigentlich kaum mehr Arbeit, die richtigen Kommentare zu haben und braucht man das nun mal sauber zusammenfügen oder vielleicht sogar automatisch das Übernehmen, was vorgeschlagen wird von den einzelnen Commits.

Andy Grunwald (00:14:28 - 00:16:17) Teilen

Wo die Informationen stehen, ist mir im Endeffekt eigentlich egal. Ich bin halt nur ein Freund davon, dass jemand mit weniger Kontext in den Pull-Request guckt und ungefähr eine Idee davon kriegen kann, was hier gerade eigentlich abgeht. Und ich denke, sobald dieses Hauptziel gegeben ist, ist eine gute Pull-Request-Description gegeben. Warum eine Pull-Request-Description? Über einer Git-Comment-Message, ganz einfach. Eine Git-Comment-Message steht halt unten irgendwo und eine Pull-Request-Description steht halt oben, direkt am Seitenanfang. Ist für mich einfacher, zugänglicher. Aber solange ein Mensch mit weniger Kontext da klarkommt, bin ich dafür. Kommen wir zu deiner Frage, wie wähle ich meine Code Reviews aus? Das ist eine wundervolle Frage. Im besten Fall hat dein Repository ein sogenanntes CodeOwners-File. Und dieses CodeOwners-File beschreibt eigentlich, welche Dateien, welche Pfade, welche Ordner von welcher zum Beispiel GitHub-Gruppe maintained werden. Das würde nämlich bedeuten, dass die Reviewer automatisch von GitHub vorgeschlagen werden. Weil dann hast du gar nicht mehr die Mühe, ordentliche Reviewer auszuwählen, weil du ab und zu ja vielleicht gar nicht weißt, wer der beste Reviewer war. Das Code-Owners-File ist da sehr hilfreich, ist aber manuell gepflegt. Es gibt auch noch die Möglichkeit ... Und Achtung, da sind wir wieder bei meiner Bachelorarbeit, Software-Repository-Mining. Es gibt auch noch die Möglichkeit, auf Basis der Git-Historie und deiner geänderten Zeilen, die Personen rauszufiltern, die diese Dateien am meisten geändert haben. Weil das sind mit hoher Wahrscheinlichkeit die Personen, die den meisten Kontext über die geänderten Zeilen haben. Dass die automatisch vorgeschlagen werden. Soviel ich weiß, macht GitHub das auch, wenn kein Codeowners-File da ist.

Wolfi Gassler (00:16:17 - 00:16:32) Teilen

Wenn du jetzt die Leute in deinem Team kennst, wen wählst du von deinem Team aus? Der, der am schnellsten reviewt? Der, der am einfachsten reviewt? Der, der immerhin schreibt, looks good to me? Den Senior, der ganz viel weiß?

Andy Grunwald (00:16:32 - 00:17:40) Teilen

Eigentlich möchte ich gar keinen auswählen. Und der Grund ist folgender. Wenn ich Leute einzeln auswähle, habe ich so eine Art Bias. Es gibt Leute, mit denen komme ich sehr, sehr gut klar im Team. Es gibt Leute, mit denen komme ich weniger gut klar. Und das ist auch okay so. Code Reviews ordentlich zu machen, kostet aber unglaublich viel Zeit. Das raubt dir Zeit von deiner Fokuszeit zu programmieren. GitHub hat so ein wundervolles Feature, da kannst du einfach deine Teamleute reingeben und GitHub Assigned dann im Load Balancer Round Robin Verfahren von deinem Team dir ein oder zwei Code-Reviewer. Somit ist die Arbeit über dein Team fair verteilt und du kriegst ordentliches Feedback, weil du halt deinen persönlichen Bias rausnimmst, um immer deinen Best Buddy zu nehmen, der deinen Code sowieso immer durchwiegt. Aber hoffentlich habt ihr nicht nur Code Reviews im Team, sondern hoffentlich macht ihr Inner Source innerhalb der Firma, nämlich die Anwendung von Open Source Best Practices innerhalb einer geschlossenen Organisation. Und hoffentlich macht ihr dann auch Cross-Team-Pull-Requests, damit ihr andere Applikationen auch modifiziert. Und dann kennt ihr ja gar nicht alle Team-Member zum Beispiel.

Wolfi Gassler (00:17:41 - 00:17:51) Teilen

Wenn jetzt so eine Code-Review-Anfrage bei mir ankommt, auf was solltet ihr denn jetzt Acht geben oder auf was solltet ihr mich konzentrieren, wenn ich so ein Code-Review schreibe?

Andy Grunwald (00:17:51 - 00:18:09) Teilen

Du meinst jetzt als Reviewer. Als erstes würde ich mir mal durchlesen, was da eigentlich geändert werden sollte. Die Beschreibung. Und um in den Code zu gucken, lese ich die Beschreibung und frage mich, ist das eigentlich ein Problem, was wir haben und sollte der Code eigentlich geändert werden? Nicht jedes Refactoring macht Sinn, nur weil das dem einen oder anderen jetzt in dem neuen Code-Style besser gefällt.

Wolfi Gassler (00:18:09 - 00:18:18) Teilen

Ich sehe schon dich als Code Reviewer zu haben, das ist eine harte Nummer. Da schreibe ich eine Woche lang Refactoring, dann kommt der Andi und sagt Bullshit, brauchen wir nicht.

Andy Grunwald (00:18:18 - 00:18:27) Teilen

Du stellst mich hier total negativ dar. Ich sage halt nur, erstens, wenn du überhaupt schon eine Woche Code bist ohne mit jemandem zu sprechen, warum du das tust, dann hast du ja sowieso schon eine ganze Menge falsch gemacht.

Wolfi Gassler (00:18:27 - 00:18:32) Teilen

Ja, du bist ja dieser komische Andi aus diesem anderen Team. Mein Team ist ja voll auf meiner Seite.

Andy Grunwald (00:18:33 - 00:18:47) Teilen

Ja und ich glaube, wir hatten mal in Folge, in Episode 1 oder 2 oder 3 irgendwie über Open Source gesprochen und da haben wir glaube ich erwähnt, bevor du erstmal eine Woche Arbeit reinsteckst, quatschst du erstmal mit dem anderen Team und holst dir erstmal ein paar Empfehlungen ab, wie man das am besten implementiert.

Wolfi Gassler (00:18:48 - 00:18:56) Teilen

Folge 4 war's, die Open-Source-Folge. Ja, sehr interessant, wie man Open-Source macht. Also, mit anderen Reden ist eine gute Möglichkeit. Auf jeden Fall, ja.

Andy Grunwald (00:18:56 - 00:20:17) Teilen

Aber um auf deine Frage zurückzukommen, natürlich sollte man sich den Code auch ansehen. Hat der irgendwelche grobe Schnitzer drin? Sind da irgendwelche Logikfehler drin? Zum Beispiel eine fehlerhafte If-Condition, wo vielleicht auf True gecheckt wird, aber es wird gerade auf False gecheckt. Sind die Variablen ordentlich benannt? Neben dem offensichtlichen, neben dem, was wirklich geändert wurde, ist meines Erachtens nach am wichtigsten das, was nicht offensichtlich ist. Und das, was nicht offensichtlich ist, ist, passen die geänderten Zeilen eigentlich genau dahin, wo sie geändert wurden oder hinzugefügt wurden? Oder sollten sie besser gar nicht in dieser Applikation sein oder in einer anderen Komponente? Was meine ich damit? Ein guter Code-Review bezieht den kompletten Kontext der Komponente, der Library oder der Applikation mit ein und fragt sich, dieses Feature, was du hier gerade eingebaut hast, soll das genau in diese Klasse oder soll das lieber woanders hin? Und mit diesem woanders hin, das bezieht sich auf andere Klassen, auf andere Dateien, die in deinem Pull-Request wohlmöglich gar nicht angefasst wurden. Das bedeutet, Der Reviewer sollte im bestmöglichsten Fall die komplette Applikation, zumindestens im groben Überblick, im Kopf haben, um zu sagen, architekturell gehört das jetzt hier nicht hin, sondern eher darüber.

Wolfi Gassler (00:20:17 - 00:21:11) Teilen

Wichtig ist meiner Meinung nach da auch, dass man dann aber einen Vorschlag bringt. Also ganz oft sind so Code Reviews irgendwie so, hast du dir das gut überlegt? Sollen wir das wirklich dort machen? Fragezeichen, that's it. Ich glaube, man sollte wirklich dann da auch mit Vorschlägen kommen, Weil das Schlimmste sind so Comments, mit denen man wenig anfangen kann. Was soll man denn jetzt wirklich ändern? So irgendwie nur so eine Frage stellen oder so. Es kann schon manchmal auch hilfreich sein, dann kann man einfach sagen, okay, na, ich bin der Meinung, das passt dort. Aber gerade so, sollten wir das nicht woanders implementieren oder das ist der falsche Platz, hilft halt dann wenig, wenn man nicht zusätzlich eine Lösungsmöglichkeit gibt. Auch das was wir letztes mal besprochen haben, da mit der Kommunikation, dass wenn man ein Kommentar in einem Dokument zum Beispiel reinsetzt, dass man da immer einen Vorschlag bringt, was man denn wirklich ändern soll und nicht nur motzt über das ganze. Motzen verstehst du übrigens, ist das deutsch genug?

Andy Grunwald (00:21:11 - 00:21:15) Teilen

Motzen verstehe ich, wird aber im Pott eigentlich kaum angewandt, ist einfach meckern.

Wolfi Gassler (00:21:16 - 00:21:18) Teilen

Oder in Wien war es Sudan.

Andy Grunwald (00:21:18 - 00:21:18) Teilen

– Sudan?

Wolfi Gassler (00:21:18 - 00:21:20) Teilen

– Sudan, ja.

Andy Grunwald (00:21:20 - 00:21:21) Teilen

– Ist das Angelegenheit an Södern?

Wolfi Gassler (00:21:21 - 00:21:28) Teilen

– Na, die wenigsten kennen Söder in Wien, glaube ich. Aber es ist vielleicht ganz passend in dem Fall, ja.

Andy Grunwald (00:21:28 - 00:22:32) Teilen

– Aber das mit dem Vorschlag, mit dem Proaktivsein unterstreiche ich. Neben dem Vorschlag bin ich auch immer ein großer Fan davon, eine Frage zu stellen, einfach um die Meinung des originalen Autors zu kriegen und nicht immer zu sagen, hey wir sollten das so und so machen weil im grunde ist wir sollten das so und so machen halt auch schon so eine art vorschlag halten direkter vorschlag vielleicht aber ich bin da immer ein fan davon ob der auto denn auch noch d'accord damit wäre, ja also immer eine frage stellen immer um feedback fragen weil im endeffekt review mein zwar den code aber die verantwortung ist immer noch beim beim originalen autor. Das Tolle ist, beim Proaktivsein geht's nicht nur um Vorschläge, lass uns den Code mal da und dahin auslagern, sondern oft haben wir zum Beispiel auch so was Einfaches wie, du schreibst in deinem Pullrequest, in deinem Code vielleicht ein paar neue Logzeilen. Und in den Logmessages hast du vielleicht irgendwo einen Tippfehler. Dann kann der Codereviewer natürlich auch sagen, hey, du hast hier einen Tippfehler, das Wort heißt nicht Butterbrot, sondern Knifte.

Wolfi Gassler (00:22:32 - 00:22:32) Teilen

Bitte was?

Andy Grunwald (00:22:33 - 00:22:36) Teilen

Knifte. Stulle, sagt dir das nichts?

Wolfi Gassler (00:22:36 - 00:22:39) Teilen

Stulle sagt mir noch was, ist irgendwie so ein Brötchen, oder?

Andy Grunwald (00:22:39 - 00:22:46) Teilen

Ja, so ein Butterbrot, ne? Eine Knifte. Super viele Leute sagen auch, das Ende von einem Brot sei eine Knifte. Das Endstück.

Wolfi Gassler (00:22:46 - 00:22:48) Teilen

Das Scherzall.

Andy Grunwald (00:22:48 - 00:22:48) Teilen

Das was?

Wolfi Gassler (00:22:48 - 00:22:50) Teilen

Das Scherzall.

Andy Grunwald (00:22:50 - 00:23:17) Teilen

Da fällt ihm selbst irgendwie der Stift runter. Auf jeden Fall proaktiv sein. Was man aber auch bei github machen kann ist man kann nicht nur ein kommentar hinschreiben hey ersetzt man butterbrot durch knifte sondern man kann auch hingehen und sagen man kann ne inline code suggestion machen dann tippt man das da ein und der autor kann einfach über neben drücken und dann wird das automatisch mitgemacht das ist natürlich. enorm super um die ganze sache auch ein bisschen bisschen zu beschleunigen.

Wolfi Gassler (00:23:17 - 00:23:23) Teilen

Kann er das dann noch irgendwie rückgängig machen oder also akzeptieren oder wird es dann automatischen kommit.

Andy Grunwald (00:23:23 - 00:23:37) Teilen

Nee das ist genauso wie bei google docs wenn du zum beispiel im vorschlag modus bist du kannst dann bei bei github hingehen okay kannst du neuen code eintippen. Und der der autor kann sagen ich habe dieses kommen in diesen code möchte ich bitte übernehmen ist natürlich sehr cool.

Wolfi Gassler (00:23:37 - 00:24:20) Teilen

Ja ich glaube man muss sich aber auch im klaren sein gerade als reviewer das nicht alles übernommen werden muss ich glaube man darf da dann auch nicht beleidigt sein wenn man irgendwie sachen gewisse sachen vorschlägt. dass halt auch nur gewisse Sachen übernommen werden, gleich wie in einem Dokument, weil ich glaube es gibt halt auch klassische subjektive Ansichten und da kann man es halt mit Lösung A oder mit Lösung B machen und wenn der Autor grundsätzlich Lösung A bevorzugt, dann ist es meiner meinung nach auch okay solange das natürlich im gewissen rahmen ist kann sie sie auch diskutieren aber man darf nicht beleidigt sein und ich glaube das ist ein großer punkt oder problem was oft entsteht bei pull request und code reviews dass sich leute wirklich in die haare bekommen.

Andy Grunwald (00:24:20 - 00:25:11) Teilen

Generelle regel ist lasst das ego vor der tür also wenn du wenn du pull request betritt sei das auto oder als reviewer. Das Ego gehört da einfach nicht hin. In den Pull Requests geht es rein um die Objektivität, um den Code, um das Problem oder das neue Feature, was wir da entweder beheben oder einfügen. Natürlich kommt es natürlich auch immer ein bisschen ganz drauf an, wie restriktiv oder wie strikt sind die Hierarchien und die Verantwortlichkeiten bei euch in der Firma. Ich weiß nicht, ob ihr irgendwelche Architekten in der Firma habt, die immer die Hand drauflegen müssen, wenn irgendwie eine neue Codezeile geändert wird oder Vielleicht seid ihr in einem etwas nicht so dynamischen umfeld und habt irgendwie eine gruppe gatekeeper die wirklich über die codebase herrschen wie irgendjemand über den über den ring aus mordor oder ähnliches.

Wolfi Gassler (00:25:11 - 00:25:13) Teilen

Der berühmte architekten ring von mordor.

Andy Grunwald (00:25:14 - 00:25:55) Teilen

Also ich kann mir schon vorstellen dass in den sehr großen strikten vielleicht etwas älteren firmen nehmen wir mal eine versicherung, dass es da schon schon ein paar architektengruppen gibt oder oder dass es immer noch firmen gibt wo du vielleicht ein zwei drei langjährige mitarbeiter hast die dann so als so eine art die nennen sich jetzt vielleicht nicht gatekeeper und sind gatekeeper. Ja, das sind dann, wie sieht sowas aus? Das sieht zum Beispiel aus, dass nur die Leute Merge-Rechte haben. Ja, zum Beispiel. Oder dass die Leute vielleicht auch einfach das Management-Backing haben, auch Änderungen komplett abzulehnen. Richtig ist das natürlich nicht, ganz und gar nicht. Ich will nur sagen, alles schon gesehen.

Wolfi Gassler (00:25:55 - 00:27:17) Teilen

Eine andere Möglichkeit, was ich bei Code Reviews auch immer wichtig finde, ist, dass man halt eben nicht wertend irgendwas reinschreibt, sondern möglichst objektiv und vielleicht eben auch dazu schreibt, dass es jetzt vielleicht kein wichtiges Kommentar ist, nur ein Vorschlag. Es gibt ja auch dieses klassische Nit für Nitpicking. Und da meine ich jetzt nicht das Nitpicking für irgendwelche Code-Formatting-Sachen, weil das kann man über irgendeinen Linter und machen und wegautomatisieren, dass das wirklich automatisch gemacht wird, sondern da geht es mir mehr darum, um solche Vorschläge, um Kleinigkeiten, die jetzt gar nicht so wichtig sind, aber wo man einfach hinschreibt, Nit und dann irgendwie einen kleinen Vorschlag für eine Änderung und derjenige kann das dann beachten oder auch nicht. Natürlich kann es auch überhand nehmen, wenn dann alles nur mehr voll von Nitt-Kommentaren ist. Da muss man natürlich auch aufpassen und da würde ich einfach den Vorschlag machen, wenn es irgendwo Probleme gibt in so einem Code Review, in der Kommunikation, Hände weg von den Kommentaren in GitHub zum Beispiel oder in dem Tool, was ihr auch immer verwendet, sondern redet einfach kurz mit den Leuten, am besten face to face, über diesen Teil. Sonst endet es nur in so einem Hin und Her und es führt einfach zu keinem Ziel. Also einfach mal direkt mit der Person reden ist, glaube ich, wirklich sinnvoller.

Andy Grunwald (00:27:17 - 00:27:26) Teilen

Letzte Woche noch so ein Fall gehabt. Beschriebene Kommunikation ist schwierig und zwar haben sich da zwei verschiedene Kulturen miteinander unterhalten aus zwei europäischen Ländern.

Wolfi Gassler (00:27:26 - 00:27:29) Teilen

Du meinst so Duisburg und Düsseldorf?

Andy Grunwald (00:27:29 - 00:30:01) Teilen

Das wäre eher Düsseldorf und Köln, aber obwohl Duisburg und Düsseldorf wäre auch, ja es ist kulturell schon echt unterschiedlich, Currywurst gegen Kö. Okay, naja auf jeden Fall war das ein klassisches Beispiel, was ich auf der Arbeit erlebt habe, wo zwei Leute einfach ganz klassisch aneinander geraten sind und danach sind die mal in Videocall gesprungen. Und haben die ganze Sache einfach geklärt. Und am Ende haben die beide gesagt, was haben wir denn da jetzt gemacht? Einfach nur, weil dort ein Franzose und ein Finne sich halt falsch verstanden haben. Wegen einer ganz klassischen, geschriebenen Kommunikation. Und der eine hat sich so gemeint, der andere hat es vielleicht ein bisschen falsch aufgefasst etc. Und nehmt euch mal lieber die Zeit und geht mal die extra Meile, springt in den Videocall und klärt die ganze Sache. Aber du hast eine super Sache angesprochen, und zwar Nitpicking. nitpicking wie viel stunden mich das schon gekostet hat nitpicking kommentare zu lesen und da ging es dann ganz klassisch um pack bitte ein komma ans ende der zeile hier sollte eine newline hin die klammer sollte bitte in dieselbe zeile und nicht in die nächste zeile bla bla bla An alle Leute, die das hören und die das auch kennen und die das vielleicht gestern noch in einem Code Review hatten. Ich bitte euch, ich bitte euch wirklich. Setzt euch jetzt mal eine Stunde hin, schreibt eine Code zur Automation, sei es ein Jenkins-File, sei es ein GitHub-Action-File und automatisiert die Scheiße einfach weg. Code-Formatting gibt's in jeder Sprache inzwischen. Bei Go heißt es GoFormit, GoFmt, bei JavaScript gibt's das bestimmt auch, bei Java auch. Und in jeder Sprache gibt's irgendwie einen Coding-Standard, Code-Formatting-Rules. Einigt euch, automatisiert den Scheiß, ja? Lasst die Kacke hinter euch. Oder Linter, JavaScript, PHP, Perl als Skriptsprachen. Oder kompilierte Sprachen, Go, Java und so weiter. Lasst die Automation über jeden Pull-Request, über jeden Branch drüber laufen, der den Kack einmal kurz durch den Linter jagt oder kompiliert, damit ihr einfach nur diese groben Schnitze raus habt. Das erspart euch so viel Energie und gibt euch einfach so viel Lebenszeit wieder. Bitte, bitte, bitte, bitte. Komm, ich wäre sogar fast soweit, ich stelle euch ein Kasten Bier dahin, wenn ihr keine Motivationen habt, das zu machen. Macht's bitte einfach. Ihr tut der Welt wirklich einen Gefallen, dass diese scheiß Nitpicking-Kommentare, ob da jetzt ein Komma oder eine Newline hingehört, endlich mal sterben. Bitte.

Wolfi Gassler (00:30:01 - 00:30:30) Teilen

Und wer jetzt sagt, er hat Angst vor diesen Style-Guide-Diskussionen. Welchen Style-Guide verwenden wir? Und die sind wirklich extrem hart. Ihr habt da auch schon extrem viele solche Diskussionen mitgemacht. Aber man muss das so sehen, ihr macht diese Diskussion einmal, und sonst macht ihr sie bei jedem Code-Review. Und bei jeder ... Kleinigkeit. Also einigt euch einmal auf einen Style Guide und dann automatisieren. Da kann ich Andi nur recht geben. Das erleichtert sehr, sehr, sehr, sehr viel.

Andy Grunwald (00:30:30 - 00:31:41) Teilen

Und wenn ihr die Fans seid, die sich so knallhart für irgendeinen Style Guide einsetzen, bitte ich euch auch diesmal, lasst dein Ego mal zu Hause. Es ist eigentlich scheißegal, wo die Klammer steht. Ihr habt vielleicht eine Woche oder anderthalb ein bisschen Schwierigkeiten durch den Code zu lesen, aber danach gewöhnt ihr euch so schnell dran. Und wenn ihr immer noch im alten Stil schreiben wollt, hat jeder Editor einen Format-on-Save-Command. Was bedeutet, ihr packt dann einfach euren Style-Guide in euren Editor rein, drückt auf Speichern und es wird auto-formatiert. Und ganz im Ernst, wenn ihr auch nicht die Arbeit machen wollt, euch ein eigenes Style-Guide auszudenken, es gibt in jeder Sprache Bereits total gute Styleguides mit guter Argumentation. Google hat davon ganz viele und die schreiben auch zu jeder Regel, warum die das genau so machen. Nehmt einfach so einen, testet den und ihr müsst ja gar nicht durch die komplette Codebase am Anfang gehen, sondern fangt einfach an und mit der Zeit gewöhnt euch dran. Schluckt es einfach runter, ob ihr die Klammer in derselben oder in der nächsten Zeile haben wollt. Es stört einfach heutzutage keinen mehr.

Wolfi Gassler (00:31:42 - 00:32:33) Teilen

Ich kann mich erinnern, als ich bei Trivago angefangen habe, hat es da so ein Onboarding gegeben für Developer. Und Torben hat damals den Vortrag gemacht zu dem Style Guide und hat so schön erklärt, es gibt sehr viele Varianten, wo man die Klammer setzt. Es gibt diese Variante, es macht fast die ganze Welt so. Dann gibt es diese Variante, es machen schon weniger. Dann gibt es diese Variante, es macht fast niemand. Und wir bei Trivago machen es ganz anders. Wir setzen die Klammer dort. Aber im Endeffekt, nach einer Woche ist man auch das gewöhnt und das hat dann jeder so gemacht. Also im Prinzip, wie du sagst, das ist eigentlich ziemlich egal, wie man das macht. Man muss sich nur einigen auf jeden Fall. Aber ich hätte jetzt mal eine ganz andere Frage. Ich habe kürzlich einen Blogpost gelesen und der war sehr interessant, der gemeint hat, Pull Requests und Code Reviews blockieren die Performance von Teams und sollten eigentlich abgeschafft werden.

Andy Grunwald (00:32:33 - 00:32:34) Teilen

Was war die Begründung?

Wolfi Gassler (00:32:34 - 00:33:58) Teilen

Wir verlinken den Artikel natürlich gern, der kann es wesentlich besser noch, als ich jetzt das zusammenfassen kann. Aber die Grundidee war eigentlich, dass wir durch diese Pull Requests, die wir mittlerweile als Silver Bullet ansehen und einfach überall akzeptieren und die ja eigentlich aus der Open Source Welt kommen, weil dort externe Code senden und man das halt überprüfen muss. dass man das eigentlich so als Silverbullet ansieht und überall implementiert und eigentlich von dem Grundgedanken von CI, von Continuous Integration immer weiter wegkommt und man diesen Grundgedanken von CI, dass man wirklich tagtäglich zu dem main branch committed dass man von dem immer weiter wegkommt und eigentlich kein ci mehr macht und es hat dann natürlich sehr viele nebenprodukte dass man zum beispiel, sich auf andere personen verlässt dass man gar nicht mehr groß nachdenkt weil man weiß es gibt noch diesen architekt der drüber schaut und die probleme, lösen wird oder mir sagen wird, was es für Probleme gibt. Man hat dann auch noch das Problem, dass man zum Beispiel angewiesen ist auf diesen Code Review, bis man merchen kann. Das heißt, es vergehen dann Tage, man wird wieder langsamer, man arbeitet vielleicht an seinem Branch ewig lang, mercht ihn nie, hat dann wieder Probleme, weil man so viel merchen muss. Also es gibt schon einige Bereiche, die durchaus auch Nachteile haben bei dem Ganzen.

Andy Grunwald (00:33:58 - 00:34:58) Teilen

Oder glaube ich sogar sehr stark, dass auch Code Reviews Nachteile haben. Doch meine Frage ist eher, ob die Vorteile nicht überwiegen. Nehmen wir mal als Beispiel, dass man nicht alleine mergen kann. Man kann einfach nicht die Codebase verändern. Das ist ja auch ein riesen vorteil besonders in umgebungen die sehr ordent getrieben wird sind ja die zum beispiel. Vielleicht im im zahlungsverkehr sind also wolfgang wenn du jetzt bei sum up oder paypal arbeiten würdest und du wärst in der lage deine bankverbindung einfach mal. in jede Zahlung einzutragen, automatisch. Ja, machst ein Pull-Request, merge den automatisch durch. Oder kommittest vielleicht direkt in den Master-Branch. Und da PayPal ja ein modernes Unternehmen ist, haben die sehr wahrscheinlich auch Continuous Deployment, wo natürlich dein Commit automatisch vom Main-Branch in Produktion geht. Also ich würde schon sagen da ist die das delay von einer halben stunde fünf stunden vierundzwanzig stunden bis dein change reviewed wird gegenüber dem finanziellen schaden für die firma ist glaube ich schon gerechtfertigt.

Wolfi Gassler (00:34:59 - 00:35:52) Teilen

Die Frage ist halt, ob dort Fehler wirklich gefunden werden und ob das nicht einfach saubere Tests eigentlich abfangen sollten, auch in der CI-CD-Pipeline, dass das wirklich sauber getestet wird. Und die Frage ist halt wirklich, vor allem wenn dann Druck auf das ganze Team kommt, dass man halt schnell einen Pull-Request einfach merchen sollte, weil ja dieses Feature wichtig ist, der Product-Owner steht in der Tür und sagt, bitte merge doch das und dann kommen die Entwickler und schreiben halt, ja, looks good to me und dann geht es durch. Also die Frage ist, findet man wirklich diese Fehler und sollte man nicht eigentlich durch Tests und automatisierte Tests am ehesten diese Probleme finden, vor allem wenn ich in der Finanzwelt unterwegs bin und weniger durch Code Reviews, weil Code Reviews, da geht es um die Architektur, da geht es um gewisse Designfragen, aber viel weniger um Bugs, meiner Meinung nach.

Andy Grunwald (00:35:53 - 00:36:10) Teilen

Ja, aber nehmen wir doch erstmal deine Bankverbindung. Du tauschst deine E-Bahn mit einer existierenden E-Bahn von PayPal. Würdest du die E-Bahn in Unit-Tests checken oder in Integration-Tests? Oder wäre die E-Bahn eigentlich in deiner Zahlungsapplikation ein Config-Parameter ähnlich wie ein Secret und ein Passwort?

Wolfi Gassler (00:36:10 - 00:36:39) Teilen

Ja, aber es muss erst jemand in einem Code-Review das checken, weil du weißt ja, wie es läuft. Keiner nimmt sich an dem Code-Review, dann schreibt man fünfmal eine Slack-Message, macht immer noch keiner. Irgendwann kommt dann der Product-Owner und sagt, bitte mach doch jetzt endlich diesen Code-Review. Man hat aber seine eigenen Feature, die man fertig machen muss. Dann setzt man sich halt hin, zehn Minuten, schaut mal schnell über den Code drüber und ja, passt schon. Merkt drei Nitpicking-Sachen an und geht schon.

Andy Grunwald (00:36:39 - 00:36:53) Teilen

Ach komm, also du willst mir jetzt sagen, wenn wir in einer Paymentfirma die Haupt-IBAN ändern, da sagt jemand looks good to me? Ach komm, also das Beispiel ist ein bisschen sehr weit an den Haaren herbeigezogen.

Wolfi Gassler (00:36:53 - 00:38:16) Teilen

Du hast natürlich auch jetzt einen sehr kritischen Bereich gewählt, aber ich würde sagen, sogar dort sollte viel über Tests abgefangen werden. Aber meines Wissens macht es auch, ich glaube Facebook macht es so und meines Wissens sogar Booking.com, da bin ich mir jetzt aber nicht ganz sicher. Die haben ein anderes Modell, dass man grundsätzlich immer auf Main merchen kann, auch als einzelner Entwickler. Aber es gibt einen späteren Code Review, also der Code Review findet statt, aber zu einem späteren Zeitpunkt, dass eben nicht gebremst wird, dass die Leute also wirklich schnell entwickeln können, testen können, auch immer möglichst up to date sind mit dem Main Branch. Dadurch auch früher auf Fehler stoßen, weil üblicherweise hat man dann irgendwelche Feature Flags, kann das testen und sieht schon frühzeitig Probleme. Hat auch weniger Probleme mit dem ganzen Merchen am Ende, weil man halt, wenn man auf einem eigenen Branch zwei Wochen arbeitet oder so, dann dieses Merch nach zwei Wochen in einem großen Team oder in einer großen Firma ist eine Katastrophe. Also auch da gibt es natürlich Vorteile. Also ich kann dem Ganzen schon was abgewinnen, wobei man natürlich dazusagen muss, auch dieser Artikel, muss ich jetzt gleich in Schutz nehmen, ist nicht gegen Code Reviews an sich, sondern vor allem gegen Pull Requests und stellt halt die Frage, brauche ich in einem Team, wo ich auch ein gewisses Vertrauen aufbauen will und den Leuten eine Verantwortung geben will, brauche ich da unbedingt noch Pull Requests oder kann ich das vielleicht auch anders lösen?

Andy Grunwald (00:38:16 - 00:39:01) Teilen

Also Code Reviews und Pull Requests haben meines Erachtens nach keine direkte Relation zum Vertrauen, weil es ist einfach nur eine Qualitätskontrolle von einer Arbeit. Jedes Luxusunternehmen, Gucci oder ähnliches, die Taschen machen, vertrauen die Mitarbeiter mit hoher Wahrscheinlichkeit auch. Bevor eine Tasche rausgeht, findet trotzdem nochmal eine Qualitätskontrolle statt. Das ist meines Erachtens nach bei einem Handwerk wie Software Engineering, sollte das ebenfalls der Fall sein. Das ist ja genauso wie, sonst hättest du keinen Bauingenieur oder ähnliches oder einen Architekten, der die Arbeit von Handwerkern bei einem Haus auch überprüft. Also meines Erachtens nach ist das nur eine reine Qualitätskontrolle und ob man direkt mit Vertrauen kommen sollte oder ob das eine direkte Relation hat, würde ich mal in Frage stellen.

Wolfi Gassler (00:39:02 - 00:39:38) Teilen

Du lagerst auf jeden Fall die Verantwortung etwas aus. Gerade ein Junior oder auch jeder andere, es muss kein Junior sein, kann auch jeder andere sein, kann sich dann schon ein bisschen rausreden, ja, es gab ja diesen Code Review, der Code Review wird schon gewisse Sachen checken, brauche ich selber weniger Verantwortung. Ich brauche ja weniger Tests, ich will eh Code Review. Also ich glaube, es triggert schon gewisse Verhalten, die man vielleicht nicht unbedingt haben will. Aber ich glaube, Code Reviews sind schon ein wichtiger Bestandteil, vor allem um einfach die Architektur zu pflegen. Die Frage ist, muss man sie mit dieser Gatekeeper-Rolle verbinden von einem Pull-Request.

Andy Grunwald (00:39:38 - 00:41:14) Teilen

Ich meine, wenn wir in einem toxischen Umfeld arbeiten, wo einzelne Änderungen auf Personen zurückgeführt werden, weil die irgendein Bug in Produktion getriggert haben, dann hoffe ich, denken alle Leute, die uns jetzt gerade zuhören, mal nach, in welchem Umfeld sie da arbeiten. Und ganz im Sinne von einer Blameless-Kultur ist es so, dass ihr als Team die Software schreibt. Natürlich trägt jeder seinen Beitrag dazu, aber nur weil jemand ein Bug in Produktion getriggert hat, dass vielleicht die Applikation offline geschossen hat, wird man ja nicht gefeuert. Hoffe ich zumindestens. Wenn doch, meldet euch gerne mal bei mir. Wir finden für euch locker eine andere Firma. weil macht euch mal bitte bewusst wofür ihr bezahlt werdet. Ihr werdet bezahlt um Software zu ändern und dass da sich ein Fehler einschleicht ist hoffentlich auch euer täglich Brot und wer dann in einer Blaming Kultur, in einer Blaming Environment arbeitet, wo genau das der Fall ist, dann solltet ihr gegebenenfalls drüber nachdenken, was anderes zu machen, beziehungsweise für jemanden anders die Software zu ändern. Und deswegen denke ich auch nicht, dass man die Verantwortung da abdrückt, weil die Verantwortung für Software, für geänderte Software, für Features, für Bugs, liegt entweder beim Team oder bei der ganzen Firma. Weil die ganze Firma ... Ich meine, eine Software erzeugt ja nicht eine Person. Und zwar an dieser Softwarekomponente, da sind mit hoher Wahrscheinlichkeit etliche Teams, Man könnte auch theoretisch sagen, auch die Verantwortung liegt auch beim Release Engineering Team, weil der Jenkins irgendwie einen Fehler gerade hatte und nicht alle meine Unit Tests ausgeführt hat, sondern nur die Hälfte.

Wolfi Gassler (00:41:14 - 00:42:37) Teilen

Es geht mir eher darum, dass man da als Entwickler dann noch das Gefühl hat, es gibt da noch jemand anderen, der das auch checkt, also brauche ich mir selber weniger Gedanken zu dem Ganzen machen. Also es geht darum, dass ich vielleicht selber weniger perfektionistisch arbeite, weil ich weiß, es gibt noch ein, zwei andere Leute, die das checken und wenn die ein Problem sehen, werden sie es mir schon sagen. Das heißt, es fördert vielleicht ein bisschen unsauberes Arbeiten. Das geht eher in diese Richtung. Aber um vielleicht jetzt auch noch das aufzulösen, weil du nicht gefragt hast, was der Blogartikelautor vorschlägt als Lösung. Weil es geht ja nicht darum, Code Reviews grundsätzlich abzuschaffen, aber er sieht halt als eine gute Lösung einfach Pair Programming oder Mob Programming, dass man quasi dieses Programmieren gemeinsam macht und das inhärente Code Review schon hat, dass wirklich zwei Leute dran arbeiten. Dann hast du nämlich schon Code Review und du blockst dieses Feature Release oder das Release an sich gar nicht durch ein Pull Request, wo dann wieder zwei Tage was liegt. sondern du probierst einfach gemeinsam an dem Code zu arbeiten, mercht möglichst oft im klassischen Sinne der CI-Definition von Extreme Programming, von dem Extreme Programming-Book, beziehungsweise der Extreme Programming-Ansicht, dass man halt wirklich täglich mercht und dann hast du das Problem auch weniger und hast auch die Code-Qualität, weil einfach zwei Leute zusammenarbeiten.

Andy Grunwald (00:42:38 - 00:43:29) Teilen

Ist ein valider Ansatz. Ich bin aber eher so ein Fan von den technischen Mitteln, die du partiell erwähnt hattest bei Facebook und Booking oder ähnliches. Und damit meine ich Canary Deployments, Blue Green Deployments, Dark Launches, Feature Flags. Also alles technische Mittel, wo du einfach deinen Change in Produktion schippen kannst. den sogenannten Blast-Radius limitieren kannst und somit deinen Bug nicht über die komplette Infrastruktur oder das komplette Kundenportfolio irgendwie verbreitest, sondern nur in dem limitierten Radius. Da bin ich ein großer Fan von. Warum? Weil dann siehst du auch mal deinen Code direkt in Produktion, bremst niemanden aus, hast ein kontrolliertes Umfeld und kannst sogar vielleicht noch mehr über deinen Code lernen, wie zum Beispiel Performance-Bottlenecks oder ähnliches. Da bin ich eher ein Fan von.

Wolfi Gassler (00:43:30 - 00:43:47) Teilen

Das brauchst du ja, also wenn du Continuous Integration machst und jetzt wirklich täglich committest oder merge mit deinem Mainbranch, dann brauchst du das sowieso, weil du kannst ja kein halbfertiges Feature jetzt wirklich in Main merchen, wenn du keine Feature Flags hast zum Beispiel. Also ich glaube, das ist sowieso Grundvoraussetzung.

Andy Grunwald (00:43:47 - 00:44:09) Teilen

Moment mal, ziehen wir mal bitte erstmal eben das Vokabular hier bitte gerade, ja. Continuous Integration sagt erstmal nur, ich baue meine Software und teste die gegen meine Unit oder Integration Test. Ich glaube, wo du hinaus möchtest, ist irgendwie sowas wie Continuous Deployment, was dann das Paket baut und automatisch den Main Branch ohne finales Approval nach Produktion schickt.

Wolfi Gassler (00:44:10 - 00:44:25) Teilen

Genau, es ist CI, CD. Ja, das stimmt. Aber CI, wenn du merkst und wenn du dann auch deployst, dann brauchst du natürlich Feature Flags, damit dein Code nicht automatisch in Production in dem Sinne läuft, wenn er noch nur halbfertig ist zum Beispiel.

Andy Grunwald (00:44:25 - 00:44:32) Teilen

Jetzt lasse ich aber auch mal den Doc da raushängen. Wobei CD, wofür steht jetzt CD bei dir? Continuous Delivery oder Continuous Deployment?

Wolfi Gassler (00:44:32 - 00:44:33) Teilen

Ich würde sagen Continuous Deployment.

Andy Grunwald (00:44:34 - 00:45:14) Teilen

Continuous Deployment ist auf jeden Fall die Methode, wo alles automatisiert ohne manuelle Freigabe nach Produktion geschippt wird. Und Continuous Delivery ist der Zwischenschritt. dass du erst deine Software baust, dann die Software testest, dann dein Deploy-Paket baust und dann irgendein Mensch auf einen roten Knopf drücken muss und dann das Release nach Produktion geht, wohingegen bei Continuous Deployment der manuelle Step rausfällt und der Main-Branch oder Master-Branch direkt nach Produktion geschippt wird. Für sowas brauchst du natürlich die technischen Hilfsmittel wie Canary Deployments, Dark Launches, Feature Flags und so weiter und so fort.

Wolfi Gassler (00:45:14 - 00:45:17) Teilen

Dr. Andi hat aufgeklärt.

Andy Grunwald (00:45:17 - 00:45:26) Teilen

Ich bin grad so ein bisschen stolz, dass ich als Bachelorrand mit Wirtschaftsinformatik einen akademischen Doktor in Datenbank ein bisschen belernen konnte.

Wolfi Gassler (00:45:27 - 00:45:33) Teilen

Ja, du glaubst ja nicht, dass wir auf der Uni wirklich jemals was von CI, CD gelernt haben. Zumindest zu meiner Zeit.

Andy Grunwald (00:45:33 - 00:45:36) Teilen

Und das macht mich grade traurig bei den ganzen Leuten, die gerade Informatik studieren.

Wolfi Gassler (00:45:37 - 00:45:50) Teilen

Lasst uns mal wissen, ob ihr im Uni-Curriculum irgendwo ICD lernt. Also bei uns war das definitiv kein Thema, aber es ist schon lange her. Bei uns war Extreme Programming so gerade das Hype-Wort damals.

Andy Grunwald (00:45:50 - 00:46:05) Teilen

Aber springen wir nochmal kurz zurück zu dem Ausbremsen. Das ist, glaube ich, ein ganz interessanter Punkt. Und zwar geht es für mich um die Review-Zeiten. Was sind so akzeptable Zeiten, wenn du einen Pull-Request machst und nach einem Code-Review fragst? Wie lange sollte es dauern, bis du mal Feedback kriegst? Was würdest du sagen aus dem Bauch heraus?

Wolfi Gassler (00:46:06 - 00:46:26) Teilen

Ja, möglichst schnell natürlich. Und ich glaube, das ist auch das Problem, weil die haben natürlich automatisch wieder Context-Switches. Ich kann nicht wirklich vielleicht mit einem anderen Projekt anfangen, muss abwarten. Man muss sich das halt im Team auch irgendwie zurechtlegen. Aber ich glaube, im Idealfall wäre natürlich, glaube ich, wenn ich eine Sekunde später ein Review hätte. Aber das ist halt in der Realität nicht möglich.

Andy Grunwald (00:46:27 - 00:46:41) Teilen

Jetzt ist mal Butter bei die Fische möglichst schnell ja ganz im Ernst für den Hasen ist möglichst schnell eine Minute für eine Schildkröte oder für ein Faultier ist möglichst schnell vielleicht ein bisschen länger. Wovon reden wir hier reden von Tag in der Woche zwei Stunden.

Wolfi Gassler (00:46:41 - 00:47:15) Teilen

Das ist ja dieses Problem ich glaube durch den Kontext switch, Wenn ich jetzt irgendwie ein anderes Ticket, ein anderes Feature, was auch immer dann anfange, da bin ich in einer anderen Entwicklung drin. Und damit ist es dann schon auch unwichtiger, ist es vier Stunden, acht Stunden oder ein, zwei Tage, muss es halt einem Product Owner erklären, warum das schon wieder zwei Tage länger dauert. Aber im Endeffekt von dem Kontext-Switch, wenn ich wieder an einem anderen Projekt arbeite, hat es dann für mich persönlich, glaube ich, wenig Einfluss, ob es ein Tag oder zwei Tage sind. Eine Woche ist vielleicht schlecht, weil da wird schon wieder vergessen, was ich überhaupt gemacht habe vor einer Woche.

Andy Grunwald (00:47:16 - 00:47:33) Teilen

Hinzu kommt auch noch, dass du dann innerhalb von einer Woche sehr wahrscheinlich fünf offene Pull-Requests hast und somit nach dem Scrum oder Kanban-Prinzip natürlich an fünf Sachen gleichzeitig arbeitest und du deinen Work-in-Progress natürlich eigentlich limitieren solltest, um Produktivität irgendwie zu liefern, oder?

Wolfi Gassler (00:47:33 - 00:47:35) Teilen

Und viel Spaß beim Zurückmerchen in der Main-Branch nach Wochen.

Andy Grunwald (00:47:36 - 00:48:28) Teilen

Zumindest bei einem großen Entwicklerteam wirst du da bestimmt den ein oder anderen Merge-Konflikt kriegen, das ist richtig. Aber genau das ist ja auch so ein zweischneidiges Schwert. Auf der einen Seite will jeder Entwickler Code-Reviews haben, also das bedeutet Reviews auf seinen Code bekommen. Auf der anderen Seite möchten ziemlich viele Entwickler aber keine Code-Reviews machen, weil das natürlich ablenkt von der fokussierten Arbeit, von der Deep Work. Obwohl natürlich schnelles Feedback für Entwickler enorm motivierend ist. Und natürlich umso mehr Kontext-Switcher zwischen den Pull-Requests reinkommen, umso mehr vergisst man natürlich auch, was man da eigentlich gemacht hat. Und das ist natürlich auch so eine schwierige Geschichte, weil durch ein langes Feedback-Cycle willst du deine Mitarbeiter oder deine Kollegen ja auch nicht demotivieren.

Wolfi Gassler (00:48:28 - 00:48:42) Teilen

Und das ist halt dann auch so eine Frage, wenn jetzt irgendwo eine Kleinigkeit fehlt, muss ich damit dieses ganze Feature blockieren oder vielleicht das Testing auch, was ich dann wirklich im Mainbranch sauber machen könnte. Das ist halt immer die Frage, ob ich dann wirklich alles blockieren will.

Andy Grunwald (00:48:43 - 00:49:14) Teilen

Aber reden wir hier wirklich von blockieren oder reden wir von verspäten? Weil blockieren hat immer so einen negativen Touch, wohingegen ja niemand, also wir sind jetzt mal in der Umgebung, wo wir keinen Gatekeeper haben. sondern einfach nur dass kein anderer entwickler zeit hatte ja also so ohne da wirklich schuld dran zu haben und ohne sondern weil bei blockieren heißt so ne ich lasse das jetzt nicht durch und das eine aktive entscheidung wohingegen nen delay einfach so ist okay wir hatten andere dinge zu tun es ist gar nicht böse gemeint.

Wolfi Gassler (00:49:15 - 00:49:44) Teilen

Also ein Klassiker ist schon in einer gewissen Weise das Blockieren, wenn, nehmen wir mal ein Beispiel, die Dokumentation ist noch nicht hundertprozentig sauber und es fehlt irgendwas bei der Dokumentation. Darf dann dieses Feature schon von irgendwelchen Testern zum Beispiel getestet werden? Kann ich das schon mit einem Feature-Flag deployen? Oder muss das warten, bis ich die Dokumente wirklich sauber abgeschlossen habe? Oder kann ich das auch später machen?

Andy Grunwald (00:49:44 - 00:49:48) Teilen

Aber wenn du mich fragst, ich würde da jetzt ein to do dran klatschen und mergen.

Wolfi Gassler (00:49:48 - 00:49:52) Teilen

Aber du klatschst auch to do's in irgendwelche exceptions, die du fängst.

Andy Grunwald (00:49:52 - 00:51:32) Teilen

Das ist korrekt. Das ist eine gute Frage. Ich glaube, das kann man gar nicht so grundsätzlich entscheiden. Ich glaube, das kommt immer Case-by-Case. Ist es halt ein mega wichtiges Feature oder ist es kleines Technical Depth oder ähnliches. Aber eine Möglichkeit, um das Problem von langen Review-Zeiten zu lösen, ist zum Beispiel, dass du Service-Level-Agreements zwischen Teams oder zwischen deinen Teammitgliedern aufsetzt. Hört sich jetzt total Enterprise-y an, aber so meine ich das gar nicht. sondern eigentlich man spricht halt mal darüber, hey, wir geben uns gegenseitig das Versprechen, dass neue Pull Requests innerhalb von zwei oder drei Tagen Feedback bekommen. Ja, ein klassischer SLA findet man zum Beispiel im Kundensupport, dass wenn du irgendwie ein Support-Ticket bei einem Cloud-Anbieter aufmachst, dass du innerhalb eines gewissen Zeitraums halt Feedback kriegst. Und warum setzt du das nicht bei Pull-Requests genauso an? Das kann man ja super tracken durch ein Grafana-Dashboard. Grafana hat sogar ein Gitter-Plugin. Puff, macht man sich ein Dashboard, wie viel Pull-Requests haben nach 24 oder 48 Stunden noch kein Kommentar zum Beispiel. Ja, das kriegt man alles durch die GitHub-API raus, alles super. Und so kann man vielleicht mal ein bisschen Zahlen draufsetzen, wie lang eigentlich Pull-Requests offen sind. Und wenn man noch einen draufsetzen möchte, kann man sogar noch einen kleinen Slackbot schreiben, der nach fünf Tagen einfach mal eine Liste an offenen Pull-Requests in euren Select-Channeln postet. Hilft das was? Weiß ich nicht, ob es die ganze Sache ein bisschen beschleunigt. Erstellt es auf jeden Fall eine gewisse Übersicht, eine gewisse Awareness und vielleicht deckt ein Problem auf, wovon man mal in der nächsten Retrospektive reden sollte. Ja, ich denke schon.

Wolfi Gassler (00:51:33 - 00:52:18) Teilen

Und siehst du, dieses Public Shaming oder wie heißt es auf Deutsch am besten, an den Pranger stellen, das macht dann einen extremen Druck und die Leute, wenn sie wissen, okay, heute um fünf wird es wieder gecheckt, ich muss noch diesen Pull Request reviewen, machen dann Looks Good to Me und dann geht dein E-Bahn Change genauso durch. Also ob diese SLAs immer funktionieren, ist glaube ich ein guter Ansatz und man muss glaube ich in dem Team auch immer gemeinsam sich irgendwie einen Prozess zurechtlegen und vielleicht auch sowas wie SLAs. Aber ich glaube, dass nicht automatisch immer dieses Messen von allem und dann das automatisierte Messen immer das Perfekte ist und alle Probleme löst, sondern wie gesagt sogar glaube ich viele Probleme wieder dadurch entstehen.

Andy Grunwald (00:52:19 - 00:53:04) Teilen

Ist es bestimmt nicht. Es ist nicht der heilige Gral. Aber bevor ihr im Team das 25. Mal zusammensitzt und euch darüber beschwert und gar nichts versucht, kann man eher in die Richtung gehen und es versuchen. Weil ich denke, etwas versuchen ist immer noch besser als nichts zu tun. Auch wenn es später fehlschlägt, heißt das nicht, dass ihr nicht erfolgreich wart. Ihr wisst nur einen weiteren Weg, wie es nicht funktioniert. Und auf der anderen Seite muss man natürlich auch über Review-Zeiten mal ein bisschen differenzieren zwischen Pull-Requests in der Firma, wo Leute dafür bezahlt werden, und Pull-Requests in Open Source. In Open Source ist es zum Beispiel super oft so, man macht ein Pull-Request und kriegt monatelang irgendwie keine Antwort, was natürlich auf der einen Seite super demotivierend ist, auf der anderen Seite aber auch irgendwie verständlich.

Wolfi Gassler (00:53:05 - 00:53:15) Teilen

Da noch einmal eingeworfen, auch Episode 4 haben wir darüber gesprochen, was man machen muss, um gut Open Source zu maintainen und wie man das Ganze angeht.

Andy Grunwald (00:53:15 - 00:53:26) Teilen

Aber in der Firma sieht es natürlich ein bisschen anders aus. Da hat man seine Teamkollegen, damit verdienen die ihr Brot. Und da kann man natürlich über Review-Zeiten auch mal sprechen.

Wolfi Gassler (00:53:27 - 00:54:33) Teilen

Um das aber auch noch zu erwähnen, also ich glaube, auch wenn man jetzt keine Pull-Requests hätte und man sagt, okay, man macht Code-Reviews später oder man erlaubt auch, dass man was released, ohne dass man jetzt die Dokumentation fertig hat oder ein gewisses Dokument, das die Architektur darstellt, was auch immer, dann muss man, glaube ich, SLAs oder eine gewisse Vereinbarung treffen, wann diese Dokumente nachgereicht werden und wie schnell. Weil sonst hat man dieses Problem, man released und dann wird es irgendwie auf die lange Bank geschoben, man vergisst es und dann entstehen diese Dokumente zum Beispiel nie, wenn man die nicht schon vorher mit einplant. Also ich glaube, das ist ganz, ganz wichtig und vor allem auch bei den Product Ownern muss das ganz, ganz klar sein, wenn das Feature released ist, dann gibt es da vielleicht noch Arbeit zu tun, sei es eine Dokumentation, sei es ein Architekturdiagramm, sei es dann eine Verbesserung von der ganzen Codebase. Da muss sich jeder im Clan sein, im Team, und das muss man auch dementsprechend kommunizieren und vereinbaren. Also, diese SLAs gibt's in beiden Varianten. Egal, ob man jetzt Pull-Requests macht oder alles direkt durchschießt mit CI, CD.

Andy Grunwald (00:54:33 - 00:55:04) Teilen

Und wenn ihr wirklich mal überlegt, solche SLAs einzuführen, dann überlegt euch bitte auch, was passiert, wenn ihr diese Agreements, diese Service-Level-Agreements, nicht einhalten könnt. Das heißt nicht, ihr müsst euch gegenseitig bestrafen oder rügen oder Ähnliches. Das reicht doch einfach nur, dass ihr dann einmal würfelt, wer dem ganzen Team Kuchen backt. Irgendeine Art von Bestrafung klingt jetzt so hart, weil ich glaube, ich würde bei Kuchen glaube ich öfter die SLA breachen, um mehr Kuchen zu kriegen.

Wolfi Gassler (00:55:05 - 00:55:22) Teilen

Das haben wir ja eh schon mal besprochen, dass du immer eine Strafe brauchst, auch bei deinem Sport, mit dem zu spät kommen brauchst du eine Strafe. Ich glaube es ist nicht jeder so, aber vielleicht ist eine Strafvereinbarung ein Bier pro Stunde, die nicht gereviewt wird oder so, vielleicht gar nicht so schlecht. Ist eine gute Idee.

Andy Grunwald (00:55:22 - 00:55:39) Teilen

Vielleicht nennen wir es nicht Strafe, vielleicht nennen wir Konsequenzen. Weil wenn man keine Konsequenzen zu selbst aufgesetzten Regeln hat, dann machen die selbst aufgesetzten Regeln irgendwie keinen Sinn oder jeder sagt so, kommst du heute, kommst du morgen. Kommst du heute nicht, kommst du morgen, meine ich.

Wolfi Gassler (00:55:39 - 00:56:17) Teilen

Ich finde die Bier-Variante oder irgendwas, was man sich im Team ausmacht, auf jeden Fall besser, als wenn es irgendein offizielles Monitoring gibt und ein Public-Shaming oder an den Pranger stellen oder irgendwelche Public-Slack-Messages. Ich glaube, so eine interne Vereinbarung ist einfach eine sympathischere Art, das Ganze umzusetzen und ich glaube, man braucht nicht immer diese Keule von irgendeinem Manager oder von sonstigen offiziellen Stellen also da muss man glaube ich auch wirklich aufpassen auch von manager seite wenn man sowas implementieren will ich glaube das team muss musste selber implementieren und und vereinbaren und da sollen auch alle glücklich damit sein.

Andy Grunwald (00:56:17 - 00:56:31) Teilen

Okay aber vielleicht ist kuchen backen jetzt nicht gerade das geilste weil dann lässt man den SLA öfter irgendwie rüberziehen vielleicht ist es irgendwie so das team muss ein killepitch trinken weil so heftigen kräuter schnappst auch nicht das geile oder vielleicht weiß ich nicht sex oder so.

Wolfi Gassler (00:56:32 - 00:56:34) Teilen

Du fangst mit den wirklich harten strafen an.

Andy Grunwald (00:56:34 - 00:56:38) Teilen

Ja also Sekt wissen wir ja alle, Blubberwasser verdirbt den Charakter.

Wolfi Gassler (00:56:39 - 00:56:40) Teilen

Da kann ich nur zustimmen.

Andy Grunwald (00:56:40 - 00:56:54) Teilen

Naja überlegt euch was, lasst uns auch mal wissen wie ihr damit umgeht. Besprecht ihr das 25 mal hintereinander in der Retrospektive, ist das ein Dauerbrenner bei euch oder wie geht ihr mit sowas um? Ich erwarte kreative Teamvorschläge.

Wolfi Gassler (00:56:54 - 00:57:27) Teilen

Und gibt es wirklich Teams, die keine Pull-Requests machen? Das würde mich wirklich interessieren. Ich habe einiges dazu gelesen, dass es durchaus Entwicklungen gibt in diese Richtung, aber würde mich interessieren, ob es wirklich da draußen in der Praxis wirklich schon Teams gibt, die das komplett überspringen, die aber trotzdem vielleicht in irgendeiner Form Codereviews machen. Also lasst uns bitte wissen, wie ihr das macht. Am besten unter Twitter, E-N-G, Kiosk ist der Twitter-Handle oder natürlich auch, wer es nicht so öffentlich haben will, stetig at engineeringkiosk.dev Und das nehmen wir, glaube ich.

Andy Grunwald (00:57:27 - 00:57:49) Teilen

Jetzt auch als gutes Schlusswort. In dieser Episode haben wir ein bisschen über Code Reviews gesprochen. Was sind die Do's, was sind die Don'ts, was sollte man als Pull-Request-Autor machen, wie kann man proaktiv sein und wie kann man ein guter Reviewer sein. Ein paar Sachen haben wir aber nicht erwähnt, wie zum Beispiel, was für Tools gibt es denn noch so, wie Pull-Request-Templates, was ist eigentlich Gerrit als Code-Review-System.

Wolfi Gassler (00:57:50 - 00:58:01) Teilen

Das lassen wir jetzt als Hausübung für unsere Hörerinnen, damit man sich auch mal einfach so weiterbilden kann, ohne dass man das Podcast immer hört. Oder wir erwähnen es einfach mal in den nächsten Episoden.

Andy Grunwald (00:58:01 - 00:58:12) Teilen

Dass Code Reviews ein schwieriges und subjektives Thema sind, haben wir, glaube ich, bewiesen, weil ich bin mal wieder froh, dass Wolfgang und ich ein Thema gefunden haben, wo wir nicht wirklich d'accord sind. Aber auch das gibt es.

Wolfi Gassler (00:58:13 - 00:58:16) Teilen

Und ausnahmsweise mal ein technisches Thema und nicht die Kunst von Kil... Kil... Kil...

Andy Grunwald (00:58:16 - 00:58:17) Teilen

Kil... Kil... Kil... Kil... Kil... Kil... Kil...

Wolfi Gassler (00:58:17 - 00:58:19) Teilen

Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil...

Andy Grunwald (00:58:19 - 00:58:20) Teilen

Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil...

Wolfi Gassler (00:58:21 - 00:58:26) Teilen

Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil... Kil...

Andy Grunwald (00:58:26 - 00:58:31) Teilen

Kil... Kil... Kil... Kil Wir kriegen auf jeden Fall kein Affiliate oder ähnliches, aber...

Wolfi Gassler (00:58:31 - 00:58:35) Teilen

Vielleicht sollten wir damit starten. Oder immer noch unsere eigenen NFTs erstellen.

Andy Grunwald (00:58:35 - 00:58:39) Teilen

Aber auch mal eine kurze Testfrage. Ist das ein Hardcore-Tech-Thema für dich gewesen?

Wolfi Gassler (00:58:39 - 00:58:43) Teilen

Mit dir ist fast jedes Thema ziemlich hart, wenn man darüber diskutiert.

Andy Grunwald (00:58:43 - 00:58:45) Teilen

Kniffte, ne?

Wolfi Gassler (00:58:45 - 00:58:50) Teilen

Ja, lassen wir das Butterbrot Butterbrot sein. Dann hören wir uns nächste Woche wieder.

Andy Grunwald (00:58:50 - 00:58:52) Teilen

Vielen Dank für's Dabeisein. Bis nächste Woche.

Wolfi Gassler (00:58:52 - 00:58:57) Teilen

Tschau. Copyright WDR 2021.