Anti-Patterns bei Code-Reviews

xguru · 2024-08-26T11:11:01+09:00

Code-Reviews scheinen eine gute Idee zu sein, oder? Durch Code-Reviews erhalten zwei Entwickler die Möglichkeit, gemeinsam auf den Code zu schauen, Probleme zu entdecken und ein Verständnis für den Entwicklungsverlauf des Projekts zu teilen Der Reviewer kann beim genauen Blick auf den Code des Autors nützliche Tricks lernen oder Gelegenheiten finden, dem Autor nützliche Tricks zu zeigen Das ist jedoch die Art und Weise, wie Reviewer der „hellen Seite“ Code-Reviews nutzen. Sie verwenden Code-Reviews, um den Code zu verbessern und die kollektiven Fähigkeiten der Entwickler zu steigern Code-Reviews können auch ein mächtiges Werkzeug für völlig andere Zwecke sein. Wenn ein Reviewer zur „dunklen Seite“ wechselt, kann er verschiedene Methoden einsetzen, um Code-Verbesserungen zu behindern oder zu verzögern Er kann auch andere persönliche Ziele verfolgen, etwa den Patch-Autor schikanieren oder vollständig entmutigen Wer kürzlich erst zur „dunklen Seite“ gewechselt ist, hat möglicherweise noch nicht an alle Möglichkeiten gedacht Deshalb stellt dieser Artikel eine Liste von Anti-Patterns für Reviewer der dunklen Seite vor [Anti-Patterns] The Death of a Thousand Round Trips Während des Lesens des Codes wird sofort bei jeder Kleinigkeit ein Review-Kommentar hinterlassen und das Lesen an dieser Stelle abgebrochen Der Entwickler behebt gewissenhaft diese Kleinigkeit und reicht den überarbeiteten Patch erneut ein Der Reviewer beginnt, diese Version zu lesen, entdeckt wieder eine andere Kleinigkeit – die er schon im ersten Review hätte erwähnen können, aber nicht erwähnt hat. Er weist auf diese Kleinigkeit hin und bricht das Lesen erneut ab Das wird wiederholt, bis der Entwickler die Hoffnung verliert The Ransom Note Dieser Patch scheint für den Entwickler besonders wichtig zu sein Für den Reviewer ist er jedoch nicht so wichtig. Daher befindet sich der Reviewer in einer Machtposition Die vom Entwickler gewünschte Änderung kann als Geisel festgehalten werden, bis zusätzliche Arbeiten erledigt sind, die dem Reviewer wichtig sind Diese zusätzlichen Arbeiten müssen nicht tatsächlich in denselben Commit aufgenommen werden, sind für den Reviewer aber wichtig The Double Team Ein Patch, zwei Reviewer Immer wenn ein Reviewer Änderungen verlangt, setzt der Entwickler sie gehorsam um. Dann ist der andere Reviewer mit dem Beschweren an der Reihe Die Reviewer wechseln sich damit ab, einander widersprechende Anforderungen zu stellen Sie kommentieren aber immer nur in Richtung des Entwicklers. Eine direkte Auseinandersetzung untereinander im Review-Thread wird vermieden Ziel ist zu sehen, wie oft die Reviewer den Entwickler hin- und herschicken können, bis er aufgibt The Guessing Game Es gibt ein Problem, und es gibt verschiedene Möglichkeiten, dieses Problem zu lösen Der Entwickler wählt eine Lösung und reicht den Patch ein Der Reviewer kritisiert genau diese Lösung aus Gründen, die nichts damit zu tun haben, ob sie das Problem löst Zum Beispiel mit dem Verweis auf einen Verstoß gegen vage Design-Prinzipien oder auf Inkompatibilität mit zukünftigen Entwicklungsplänen Wenn die Kritik vage genug bleibt, weiß der Entwickler nicht, wie der bestehende Patch geändert werden müsste, um die Kritik auszuräumen Dann ist es für den Entwickler am sinnvollsten, zur Lösung des ursprünglichen Problems eine völlig andere Lösung zu wählen und stattdessen zu implementieren Daraufhin lehnt der Reviewer auch diese wieder in derselben unhilfreichen Weise ab The Priority Inversion Im ersten Code-Review werden nur kleine, einfache Dinge beanstandet Es wird gewartet, bis der Entwickler sie behoben hat, und erst dann wird ein schwerwiegendes Problem angesprochen Es gibt ein grundlegendes Problem, das eine vollständige Neuschreibung eines erheblichen Teils des Patches erfordert. Das bedeutet, dass ein großer Teil der bereits erledigten kleinen Korrekturen verworfen werden muss Kaum etwas vermittelt besser die Botschaft „Deine Arbeit ist nicht gewollt, und deine Zeit ist nicht wertvoll“, als jemanden viel Arbeit machen zu lassen und sie dann wegzuwerfen Das allein kann schon ausreichen, damit der Entwickler aufgibt The Late-Breaking Design Review Über Wochen oder Monate wurde an einer enorm komplexen Arbeit gearbeitet, aufgeteilt in viele separate Patches Der Reviewer ist mit dem Design dieser Gesamtarbeit nicht einverstanden, war aber an der ursprünglichen Diskussion nicht beteiligt oder gehörte in der Diskussion zur unterlegenen Seite Nun wird der Reviewer jedoch gebeten, einen kleinen, aber wichtigen Teil mitten in dieser Arbeit zu prüfen (zum Beispiel einen einfachen Fix für einen Bug, der viele Entwickler blockiert). Das ist für den Reviewer eine Gelegenheit Der Reviewer verlangt nun eine Rechtfertigung für das gesamte Design der bisherigen Arbeit Wenn der Entwickler nur für einen Teil der Gesamtarbeit verantwortlich ist und die Antwort nicht kennt, ist das nicht das Problem des Reviewers. Der Reviewer wird den „Approve“-Button nicht drücken, bis er zufrieden ist The Catch-22 Ein großer Patch ist zu schwer zu lesen. Es wird verlangt, ihn in in sich geschlossene Unterteile aufzuteilen Umgekehrt gilt: Gibt es viele kleine Patches, dann seien einige davon für sich genommen bedeutungslos. Es wird verlangt, sie wieder zusammenzuführen Wenn es den Anschein hat, dass bei einem konkreten Fall irgendeine Art von Trade-off relevant sein könnte, lässt sich das als Anlass für Beschwerden nutzen Wenn der Code zum Beispiel gut verständlich geschrieben ist, wird er wohl schlecht performen; und wenn er gut optimiert ist, wird er schwer zu lesen und zu warten sein The Flip Flop Es gibt eine Art von Änderungen, die üblicherweise immer wieder an demselben Teil des Codes vorgenommen werden Der Reviewer hat solche Änderungen schon mehrfach geprüft Wieder kommt eine weitere Änderung herein. Der Autor hat sich die früheren Änderungen genau angesehen, das bestehende Muster sorgfältig befolgt und einen Reviewer ausgewählt, der mit diesem Bereich vertraut zu sein scheint Der Reviewer erhebt plötzlich Einwände gegen einen Aspekt der Änderung, den er zuvor nie beanstandet hat. Es reicht nicht mehr aus, dem bestehenden Muster einfach zu folgen Was passiert, wenn der Autor auf dieselbe Änderung in früheren Fällen hinweist und die Inkonsistenz des Reviewers anspricht? Der Reviewer sagt: „Stimmt. Das sollte auch geändert werden.“ Der Reviewer muss darauf achten, sich nicht freiwillig zu melden, alle bestehenden Fälle selbst zu ändern. Mit etwas Glück interpretiert der Entwickler das als Anweisung, die vorhandenen Fälle eigenständig zu ändern, was dem Reviewer viel Arbeit erspart Aber im Ernst ... Bis hierher war dieser Artikel ein Scherz. Er will nicht andeuten, dass Reviewer sich absichtlich so schlecht verhalten Die meisten Beschreibungen sind Übertreibungen dessen, was Reviewer tatsächlich tun, oder Übertreibungen dessen, wie frustrierte Patch-Einreicher es empfinden In Wirklichkeit sollte man genau das nicht tun! Man sollte versuchen, Round Trips zu minimieren, wichtige Neuschreibungen (falls nötig) anzusprechen, bevor man an Kleinigkeiten herumkrittelt, und beim Kritisieren eines Patches konstruktive Vorschläge dazu machen, welche Version man akzeptieren würde Wenn man eine Meinung zur gesamten Codebasis hat, sollte man eine allgemeine Diskussion mit allen Entwicklern führen, statt sie beim Review des Patches eines einzelnen Entwicklers als Vorwand für Nörgelei zu benutzen Wenn man so etwas versehentlich getan hat, sollte man es erkennen. Man sollte wahrnehmen, dass man das Leben eines Entwicklers unbeabsichtigt schwerer gemacht hat, sich entschuldigen und sich bemühen, hilfreicher zu sein Das grundlegende Problem ist Machtmissbrauch Wenn ein Entwickler zum Code-Reviewer für den Patch eines anderen Entwicklers wird, erhält er vorübergehend Macht. Er hat die Macht, zu verhindern, dass dieser Patch committed wird Mit Macht geht Verantwortung einher. Man sollte sie nur für den beabsichtigten Zweck und nur im nötigen Maß einsetzen Die meisten Anti-Patterns (oder die milderen Verhaltensweisen, die hier satirisch überzeichnet werden) sind Machtmissbrauch. Der Reviewer nutzt seine vorübergehende Macht über den Entwickler als Hebel, um persönliche Ziele zu verfolgen, die mit der Verbesserung des Patches nichts zu tun haben oder ihr sogar entgegenstehen Diese persönlichen Ziele unterscheiden sich je nach Anti-Pattern. Es können irrelevante Arbeiten, persönliche Stilvorlieben, Faulheit, Widerstand gegen Veränderungen oder persönliche Abneigung gegen den Patch-Einreicher sein Wenn der Patch-Einreicher in der Vergangenheit selbst als Code-Reviewer solches Fehlverhalten gezeigt hat, könnte die Abneigung gerechtfertigt sein. Deshalb sollte man die Macht eines Code-Reviewers vorsichtig einsetzen Wenn Entwickler in einen Teufelskreis gegenseitiger Vergeltung geraten, ist ein Softwareprojekt dem Untergang geweiht Gatekeeping-Code-Review Bisher lag der Schwerpunkt auf Code-Reviews unter Gleichrangigen Code-Reviewer und Patch-Einreicher sind Kollegen; sie tragen nicht füreinander Verantwortung und haben keine endgültige Entscheidungsgewalt über die Codebasis Deshalb ist die Macht, die man im Code-Review erhält, nur vorübergehend In einer Peer-Review-Situation sollten Code-Reviewer und Autor grundsätzlich dieselben Ziele haben Wenn es ernsthafte Meinungsverschiedenheiten darüber gibt, welche Funktion aufgenommen werden soll, wie stark etwas vor der Freigabe ausgereift sein muss oder welcher Coding-Style akzeptabel ist, sollte das außerhalb des Kontexts eines Code-Reviews behandelt werden In anderen Arten von Code-Review-Situationen ist das jedoch nicht so. Besonders anders ist es, wenn Außenstehende eines Projekts unaufgefordert Patches einsenden Solche Szenarien treten typischerweise bei freier Software auf Denn jeder auf der Welt kann den Source Code ändern, und manche versuchen dann, ihre Änderungen zurückzuschicken Es kann aber auch in anderen Situationen vorkommen Innerhalb eines Unternehmens, das proprietären Code entwickelt, kann ein Entwickler aus einem Team einen Patch an das Projekt eines anderen Teams schicken und auf Glück hoffen In solchen Fällen ist es viel wahrscheinlicher, dass der Empfänger des Patches die Änderung von vornherein gar nicht will oder mit ihrer Umsetzung überhaupt nicht einverstanden ist und den Patch daher gar nicht annehmen möchte In diesem Fall handelt es sich nicht um den Missbrauch vorübergehender, als Peer-Reviewer verliehener Macht, sondern um die legitime Ausübung dauerhafter Autorität als Verantwortlicher des Projekts Ich entscheide über die Richtung meines Softwareprojekts Wenn ein Code-Reviewer eine solche „Gatekeeping“-Rolle hat, ist es nicht immer falsch, einen Patch mit der Begründung zu kritisieren, er verletze bestehende allgemeine Design-Prinzipien oder Anforderungen Möglicherweise weiß der Reviewer schlicht nicht, wie sich das betreffende Problem auf eine Weise lösen lässt, die mit den Anforderungen vereinbar ist Tatsächlich könnte genau das der schwierige Teil sein und der einzige Grund, warum ich dieselbe Änderung bisher noch nicht selbst vorgenommen habe Aber auch im Gatekeeping-Kontext ist es unhöflich, „The Guessing Game“ ohne Erklärung anzuwenden Ich versuche normalerweise zu erklären, dass der Entwickler den schwierigen Teil übersehen hat, und wenn ich selbst die Antwort nicht kenne, sage ich das auch Natürlich gibt es keine Entschuldigung für passiv-aggressive Behinderung wie „The Death of a Thousand Round Trips“ Wenn man einen Patch wirklich überhaupt nicht in den Code aufnehmen will und in einer Gatekeeping-Rolle die legitime Autorität hat, diese Entscheidung zu treffen, dann kann man das auch in Worten sagen, damit der Einreicher nicht weiter seine Zeit verschwendet Disclaimer Ich habe über Jahre hinweg Notizen für diesen Artikel gesammelt – aus Code-Reviews, an denen ich selbst beteiligt war (auf beiden Seiten), aus Code-Reviews, die ich zwischen anderen beobachtet habe, und aus Code-Reviews, von denen ich nur in Gesprächen gehört habe Er richtet sich daher nicht gegen eine bestimmte Person, die kürzlich meinen Code reviewt hat

(chiark.greenend.org.uk)

61 Punkte von xguru 2024-08-26 | 13 Kommentare | Auf WhatsApp teilen

Code-Reviews scheinen eine gute Idee zu sein, oder?
- Durch Code-Reviews erhalten zwei Entwickler die Möglichkeit, gemeinsam auf den Code zu schauen, Probleme zu entdecken und ein Verständnis für den Entwicklungsverlauf des Projekts zu teilen
- Der Reviewer kann beim genauen Blick auf den Code des Autors nützliche Tricks lernen oder Gelegenheiten finden, dem Autor nützliche Tricks zu zeigen
Das ist jedoch die Art und Weise, wie Reviewer der „hellen Seite“ Code-Reviews nutzen. Sie verwenden Code-Reviews, um den Code zu verbessern und die kollektiven Fähigkeiten der Entwickler zu steigern
Code-Reviews können auch ein mächtiges Werkzeug für völlig andere Zwecke sein. Wenn ein Reviewer zur „dunklen Seite“ wechselt, kann er verschiedene Methoden einsetzen, um Code-Verbesserungen zu behindern oder zu verzögern
- Er kann auch andere persönliche Ziele verfolgen, etwa den Patch-Autor schikanieren oder vollständig entmutigen
Wer kürzlich erst zur „dunklen Seite“ gewechselt ist, hat möglicherweise noch nicht an alle Möglichkeiten gedacht
- Deshalb stellt dieser Artikel eine Liste von Anti-Patterns für Reviewer der dunklen Seite vor

[Anti-Patterns]

The Death of a Thousand Round Trips

Während des Lesens des Codes wird sofort bei jeder Kleinigkeit ein Review-Kommentar hinterlassen und das Lesen an dieser Stelle abgebrochen
Der Entwickler behebt gewissenhaft diese Kleinigkeit und reicht den überarbeiteten Patch erneut ein
Der Reviewer beginnt, diese Version zu lesen, entdeckt wieder eine andere Kleinigkeit – die er schon im ersten Review hätte erwähnen können, aber nicht erwähnt hat. Er weist auf diese Kleinigkeit hin und bricht das Lesen erneut ab
Das wird wiederholt, bis der Entwickler die Hoffnung verliert

The Ransom Note

Dieser Patch scheint für den Entwickler besonders wichtig zu sein
Für den Reviewer ist er jedoch nicht so wichtig. Daher befindet sich der Reviewer in einer Machtposition
Die vom Entwickler gewünschte Änderung kann als Geisel festgehalten werden, bis zusätzliche Arbeiten erledigt sind, die dem Reviewer wichtig sind
- Diese zusätzlichen Arbeiten müssen nicht tatsächlich in denselben Commit aufgenommen werden, sind für den Reviewer aber wichtig

The Double Team

Ein Patch, zwei Reviewer
Immer wenn ein Reviewer Änderungen verlangt, setzt der Entwickler sie gehorsam um. Dann ist der andere Reviewer mit dem Beschweren an der Reihe
Die Reviewer wechseln sich damit ab, einander widersprechende Anforderungen zu stellen
- Sie kommentieren aber immer nur in Richtung des Entwicklers. Eine direkte Auseinandersetzung untereinander im Review-Thread wird vermieden
Ziel ist zu sehen, wie oft die Reviewer den Entwickler hin- und herschicken können, bis er aufgibt

The Guessing Game

Es gibt ein Problem, und es gibt verschiedene Möglichkeiten, dieses Problem zu lösen
Der Entwickler wählt eine Lösung und reicht den Patch ein
Der Reviewer kritisiert genau diese Lösung aus Gründen, die nichts damit zu tun haben, ob sie das Problem löst
- Zum Beispiel mit dem Verweis auf einen Verstoß gegen vage Design-Prinzipien oder auf Inkompatibilität mit zukünftigen Entwicklungsplänen
Wenn die Kritik vage genug bleibt, weiß der Entwickler nicht, wie der bestehende Patch geändert werden müsste, um die Kritik auszuräumen
- Dann ist es für den Entwickler am sinnvollsten, zur Lösung des ursprünglichen Problems eine völlig andere Lösung zu wählen und stattdessen zu implementieren
Daraufhin lehnt der Reviewer auch diese wieder in derselben unhilfreichen Weise ab

The Priority Inversion

Im ersten Code-Review werden nur kleine, einfache Dinge beanstandet
Es wird gewartet, bis der Entwickler sie behoben hat, und erst dann wird ein schwerwiegendes Problem angesprochen
- Es gibt ein grundlegendes Problem, das eine vollständige Neuschreibung eines erheblichen Teils des Patches erfordert. Das bedeutet, dass ein großer Teil der bereits erledigten kleinen Korrekturen verworfen werden muss
- Kaum etwas vermittelt besser die Botschaft „Deine Arbeit ist nicht gewollt, und deine Zeit ist nicht wertvoll“, als jemanden viel Arbeit machen zu lassen und sie dann wegzuwerfen
Das allein kann schon ausreichen, damit der Entwickler aufgibt

The Late-Breaking Design Review

Über Wochen oder Monate wurde an einer enorm komplexen Arbeit gearbeitet, aufgeteilt in viele separate Patches
Der Reviewer ist mit dem Design dieser Gesamtarbeit nicht einverstanden, war aber an der ursprünglichen Diskussion nicht beteiligt oder gehörte in der Diskussion zur unterlegenen Seite
Nun wird der Reviewer jedoch gebeten, einen kleinen, aber wichtigen Teil mitten in dieser Arbeit zu prüfen (zum Beispiel einen einfachen Fix für einen Bug, der viele Entwickler blockiert). Das ist für den Reviewer eine Gelegenheit
Der Reviewer verlangt nun eine Rechtfertigung für das gesamte Design der bisherigen Arbeit
Wenn der Entwickler nur für einen Teil der Gesamtarbeit verantwortlich ist und die Antwort nicht kennt, ist das nicht das Problem des Reviewers. Der Reviewer wird den „Approve“-Button nicht drücken, bis er zufrieden ist

The Catch-22

Ein großer Patch ist zu schwer zu lesen. Es wird verlangt, ihn in in sich geschlossene Unterteile aufzuteilen
Umgekehrt gilt: Gibt es viele kleine Patches, dann seien einige davon für sich genommen bedeutungslos. Es wird verlangt, sie wieder zusammenzuführen
Wenn es den Anschein hat, dass bei einem konkreten Fall irgendeine Art von Trade-off relevant sein könnte, lässt sich das als Anlass für Beschwerden nutzen
- Wenn der Code zum Beispiel gut verständlich geschrieben ist, wird er wohl schlecht performen; und wenn er gut optimiert ist, wird er schwer zu lesen und zu warten sein

The Flip Flop

Es gibt eine Art von Änderungen, die üblicherweise immer wieder an demselben Teil des Codes vorgenommen werden
Der Reviewer hat solche Änderungen schon mehrfach geprüft
Wieder kommt eine weitere Änderung herein. Der Autor hat sich die früheren Änderungen genau angesehen, das bestehende Muster sorgfältig befolgt und einen Reviewer ausgewählt, der mit diesem Bereich vertraut zu sein scheint
Der Reviewer erhebt plötzlich Einwände gegen einen Aspekt der Änderung, den er zuvor nie beanstandet hat. Es reicht nicht mehr aus, dem bestehenden Muster einfach zu folgen
Was passiert, wenn der Autor auf dieselbe Änderung in früheren Fällen hinweist und die Inkonsistenz des Reviewers anspricht?
- Der Reviewer sagt: „Stimmt. Das sollte auch geändert werden.“
- Der Reviewer muss darauf achten, sich nicht freiwillig zu melden, alle bestehenden Fälle selbst zu ändern. Mit etwas Glück interpretiert der Entwickler das als Anweisung, die vorhandenen Fälle eigenständig zu ändern, was dem Reviewer viel Arbeit erspart

Aber im Ernst ...

Bis hierher war dieser Artikel ein Scherz. Er will nicht andeuten, dass Reviewer sich absichtlich so schlecht verhalten
- Die meisten Beschreibungen sind Übertreibungen dessen, was Reviewer tatsächlich tun, oder Übertreibungen dessen, wie frustrierte Patch-Einreicher es empfinden
In Wirklichkeit sollte man genau das nicht tun!
- Man sollte versuchen, Round Trips zu minimieren, wichtige Neuschreibungen (falls nötig) anzusprechen, bevor man an Kleinigkeiten herumkrittelt, und beim Kritisieren eines Patches konstruktive Vorschläge dazu machen, welche Version man akzeptieren würde
- Wenn man eine Meinung zur gesamten Codebasis hat, sollte man eine allgemeine Diskussion mit allen Entwicklern führen, statt sie beim Review des Patches eines einzelnen Entwicklers als Vorwand für Nörgelei zu benutzen
- Wenn man so etwas versehentlich getan hat, sollte man es erkennen. Man sollte wahrnehmen, dass man das Leben eines Entwicklers unbeabsichtigt schwerer gemacht hat, sich entschuldigen und sich bemühen, hilfreicher zu sein
Das grundlegende Problem ist Machtmissbrauch
- Wenn ein Entwickler zum Code-Reviewer für den Patch eines anderen Entwicklers wird, erhält er vorübergehend Macht. Er hat die Macht, zu verhindern, dass dieser Patch committed wird
- Mit Macht geht Verantwortung einher. Man sollte sie nur für den beabsichtigten Zweck und nur im nötigen Maß einsetzen
- Die meisten Anti-Patterns (oder die milderen Verhaltensweisen, die hier satirisch überzeichnet werden) sind Machtmissbrauch. Der Reviewer nutzt seine vorübergehende Macht über den Entwickler als Hebel, um persönliche Ziele zu verfolgen, die mit der Verbesserung des Patches nichts zu tun haben oder ihr sogar entgegenstehen
- Diese persönlichen Ziele unterscheiden sich je nach Anti-Pattern. Es können irrelevante Arbeiten, persönliche Stilvorlieben, Faulheit, Widerstand gegen Veränderungen oder persönliche Abneigung gegen den Patch-Einreicher sein
Wenn der Patch-Einreicher in der Vergangenheit selbst als Code-Reviewer solches Fehlverhalten gezeigt hat, könnte die Abneigung gerechtfertigt sein. Deshalb sollte man die Macht eines Code-Reviewers vorsichtig einsetzen
- Wenn Entwickler in einen Teufelskreis gegenseitiger Vergeltung geraten, ist ein Softwareprojekt dem Untergang geweiht

Gatekeeping-Code-Review

Bisher lag der Schwerpunkt auf Code-Reviews unter Gleichrangigen
- Code-Reviewer und Patch-Einreicher sind Kollegen; sie tragen nicht füreinander Verantwortung und haben keine endgültige Entscheidungsgewalt über die Codebasis
- Deshalb ist die Macht, die man im Code-Review erhält, nur vorübergehend
In einer Peer-Review-Situation sollten Code-Reviewer und Autor grundsätzlich dieselben Ziele haben
- Wenn es ernsthafte Meinungsverschiedenheiten darüber gibt, welche Funktion aufgenommen werden soll, wie stark etwas vor der Freigabe ausgereift sein muss oder welcher Coding-Style akzeptabel ist, sollte das außerhalb des Kontexts eines Code-Reviews behandelt werden
In anderen Arten von Code-Review-Situationen ist das jedoch nicht so. Besonders anders ist es, wenn Außenstehende eines Projekts unaufgefordert Patches einsenden
- Solche Szenarien treten typischerweise bei freier Software auf
- Denn jeder auf der Welt kann den Source Code ändern, und manche versuchen dann, ihre Änderungen zurückzuschicken
Es kann aber auch in anderen Situationen vorkommen
- Innerhalb eines Unternehmens, das proprietären Code entwickelt, kann ein Entwickler aus einem Team einen Patch an das Projekt eines anderen Teams schicken und auf Glück hoffen
In solchen Fällen ist es viel wahrscheinlicher, dass der Empfänger des Patches die Änderung von vornherein gar nicht will oder mit ihrer Umsetzung überhaupt nicht einverstanden ist und den Patch daher gar nicht annehmen möchte
- In diesem Fall handelt es sich nicht um den Missbrauch vorübergehender, als Peer-Reviewer verliehener Macht, sondern um die legitime Ausübung dauerhafter Autorität als Verantwortlicher des Projekts
- Ich entscheide über die Richtung meines Softwareprojekts
Wenn ein Code-Reviewer eine solche „Gatekeeping“-Rolle hat, ist es nicht immer falsch, einen Patch mit der Begründung zu kritisieren, er verletze bestehende allgemeine Design-Prinzipien oder Anforderungen
- Möglicherweise weiß der Reviewer schlicht nicht, wie sich das betreffende Problem auf eine Weise lösen lässt, die mit den Anforderungen vereinbar ist
- Tatsächlich könnte genau das der schwierige Teil sein und der einzige Grund, warum ich dieselbe Änderung bisher noch nicht selbst vorgenommen habe
Aber auch im Gatekeeping-Kontext ist es unhöflich, „The Guessing Game“ ohne Erklärung anzuwenden
- Ich versuche normalerweise zu erklären, dass der Entwickler den schwierigen Teil übersehen hat, und wenn ich selbst die Antwort nicht kenne, sage ich das auch
Natürlich gibt es keine Entschuldigung für passiv-aggressive Behinderung wie „The Death of a Thousand Round Trips“
- Wenn man einen Patch wirklich überhaupt nicht in den Code aufnehmen will und in einer Gatekeeping-Rolle die legitime Autorität hat, diese Entscheidung zu treffen, dann kann man das auch in Worten sagen, damit der Einreicher nicht weiter seine Zeit verschwendet

Disclaimer

Ich habe über Jahre hinweg Notizen für diesen Artikel gesammelt – aus Code-Reviews, an denen ich selbst beteiligt war (auf beiden Seiten), aus Code-Reviews, die ich zwischen anderen beobachtet habe, und aus Code-Reviews, von denen ich nur in Gesprächen gehört habe
Er richtet sich daher nicht gegen eine bestimmte Person, die kürzlich meinen Code reviewt hat

13 Kommentare

intajon 2025-02-17

Das ist überraschenderweise vielleicht keine Übertreibung....

magulhalmom 2024-09-02

Das habe ich wirklich selbst erlebt, und ich hätte deswegen fast aufgehört, Entwickler zu sein. Es war wirklich schwer, mich davon wieder zu erholen.

bungker 2024-08-28

Beim Lesen des Artikels hätte ich fast PTSD bekommen.

kayws426 2024-08-28

Offenbar haben Sie die Notizen für diesen Artikel in der Zwischenzeit gut gesammelt!!

gongsil1212 2024-08-27

Schon das Lesen davon ist auf dem Niveau psychischer Misshandlung ...

wedding 2024-08-27

Die letzte Zeile ist also der Kern, oder? lol ...

jypark 2024-08-27

Wow … ich dachte schon, ich bekomme Krebs ;;

ahwjdekf 2024-08-26

So etwas sieht man auch in Korea häufig, wenn man mal in Läden unterwegs ist, die SI + SM machen. Umgangssprachlich nennt man das oft Revierverhalten. Manche unangenehmen Leute treiben allerlei Dinge, nur um ihre Pfründe zu sichern.

kenuheo 2024-08-26

Es gibt ja viele üble Methoden.

gooksangom 2024-08-26

Langfristig gilt: Wer sich ohne vernünftigen Grund so verhält, wird entweder 1) früh aus den Entwicklernetzwerken ausgeschlossen oder 2) bleibt trotz miserablem Charakter nur deshalb im Spiel, weil die Person fachlich so stark ist, dass sie einen großen Teil trägt und schwer auszuschließen ist, und jemand die Rolle eines Adapters übernimmt, also als Kommunikationskanal fungiert und so die Verbindung aufrechterhält. Verschwindet diese Zwischenperson aus irgendeinem Grund, wird die betreffende Person in absehbarer Zeit ebenfalls schnell ausgeschlossen. Man kann sich noch so sehr abstrampeln: Am Ende müssen Menschen zusammenkommen und etwas auf die Beine stellen, damit Geld fließt, und nur wenn Geld fließt, kommen auch Menschen zusammen. Wer also nicht gut mit anderen auskommt, wird aus der Gruppe ausgeschlossen und fällt zurück.

Oft bilden sich Menschen, die innerhalb einer Gruppe trotz schlechtem Charakter lange überlebt haben, ein, sie hätten sich gehalten, weil sie irgendetwas Großartiges seien, so eine Art hochfunktionaler Soziopath wie in einem Sherlock-Drama. In Wirklichkeit erträgt das Umfeld sie nur stillschweigend, solange sie nützlich sind, und nutzt sie eben mit. Sobald dieser Nutzwert verschwindet, wird daraus schnell ein Verhältnis nach dem Motto: "Es war schmutzig, mit dir zusammenzuarbeiten, und sehen wir uns nie wieder." Selbst Sherlock mit Cumberbatch in der Hauptrolle ist für uns von außen nur ein charismatischer Soziopath; hätte es nicht Menschen um ihn herum gegeben, die ihn nicht aufgegeben und sich um ihn gekümmert hätten, gäbe es überhaupt keine Geschichte.

dokdok 2024-08-28

Bei Menschen mit schlechtem Charakter, die trotzdem lange überlebt haben, ist es oft so, dass sie dem Irrtum verfallen, sie hätten nur deshalb durchgehalten, weil sie so etwas Großartiges wie ein High-Functioning-Soziopath à la Sherlock aus irgendeinem Drama seien. In Wirklichkeit werden sie nur deshalb von ihrem Umfeld zähneknirschend ertragen und ausgenutzt, weil sie noch einen Nutzen haben; fällt dieser Nutzen weg, endet die Beziehung schnell bei „Es war widerlich, mit dir zu tun zu haben, und lass uns einander nie wiedersehen.“ ==> Das ist eine umwerfende Formulierung. Die sollte ich mir merken.

savvykang 2024-08-26

Das erinnert mich an Mobbing am Arbeitsplatz oder Power Harassment.

xguru 2024-08-26

Es soll zwar humorvoll sein, aber beim Lesen dieses Textes steigt sofort der Frust hoch..