Was tun, wenn der Entwickler, dessen Code ich überprüfe, defensiv wird?

Ich denke, wenn Ihr Tech-Lead definitiv nicht hinter Ihnen steht, müssen Sie Ihr Verhalten ändern.

Dies ist die traurige Realität des Lebens in einem Unternehmen.

Das Beste, was Sie tun können, ist, Ihren technischen Leiter zu bitten, Ihnen Richtlinien zu geben, wie Ihre Code-Reviews aussehen sollten, und sich an diese Richtlinien zu halten. Wenn der Entwickler dann seinen Rücken zu etwas bekommt, das Sie in einer Überprüfung gesagt haben, können Sie zu Ihrem technischen Leiter gehen und ihn bitten, Sie auf der Grundlage seiner Richtlinien zu unterstützen .

Ich war auf beiden Seiten dieser Gleichung, und obwohl es schwierig ist, genau zu sagen, was passiert, ohne die beteiligten Personen zu kennen oder wie sie die Dinge sehen, klingt es so, als müssten Sie hier etwas taktvoller sein. Wenn ich raten müsste (und es ist nur eine Vermutung), wird Ihr Kollege wahrscheinlich defensiv, weil er sich angegriffen fühlt. Für dich ist es ein unpersönlicher Teil des Jobs, für sie bist du ein Idiot, der sauer auf sie wird, weil sie es nicht so machen, wie du es für richtig hältst.

Allerdings könnte ich hier von der Basis abweichen. Das erste , was Sie tun sollten, ist, mit ihnen darüber zu sprechen. Dieses Gespräch muss nicht super formell sein, aber es sollte privat sein. Lassen Sie sie wissen, dass Sie versuchen, ihnen zu helfen, sich zu verbessern, und dass Ihre Kritik nicht persönlich ist. Konzentrieren Sie sich auch nicht nur auf das Negative. Wenn sie darauf eine Antwort haben, hören Sie ihnen zu und versuchen Sie, ihren Standpunkt zu verstehen. Doch selbst wenn bei diesem Gespräch nichts herauskommt, gibt es Dinge, die Sie tun können.

Verfügt Ihr Unternehmen über formelle Kodex-Standards? Sind diese Standards dokumentiert? Wenn ja, dann ist Ihr Leben gerade viel einfacher geworden. Weisen Sie jedes Mal, wenn Sie etwas sehen, das gegen einen dieser Standards verstößt, höflich darauf hin. Wenn er wütend wird, kannst du sogar sagen: "Hey, ich stimme zu, es ist ein dummer Standard, aber ich mache nicht die Regeln". Du kannst es sogar so aussehen lassen, als ob du ihm den Rücken frei hältst, indem du sagst: „Hey Alter, ich möchte nicht, dass du Ärger bekommst, du solltest das wahrscheinlich ändern, damit es dem Standard entspricht“.

Die andere große Sache, die Sie tun können, ist zu erklären, warum etwas geändert werden sollte. Seien Sie nicht einfach wie "Hey, ändere X zu Y". Erklären Sie ihm, dass X im realistischen Szenario Z Fehler verursacht oder dass Y sauberer ist und den Code leichter wartbar macht. Versuchen Sie erneut, dies so zu formulieren, dass Ihr Kollege das Gefühl hat, dass Sie aktiv versuchen, ihm zu helfen, sich zu verbessern, anstatt ihn nur zu tadeln, weil er die Dinge auf seine Weise und nicht auf Ihre Weise getan hat.

Weisen Sie auf die guten Dinge in ihrem Code sowie auf die schlechten hin. Wenn sie etwas auf intelligente Weise gemacht haben, dann sagen Sie ihnen, dass Ihnen die Art und Weise gefallen hat, wie sie die Sache gemacht haben. Das hilft zu verhindern, dass sie das Gefühl haben, dass du hinter ihnen her bist.

Der Entwickler schreibt eine Klasse, die wie etwas zwischen Factory und Container aussieht, und nennt sie dann SomethingBuilder, wobei der Something-Teil nichts mit dem endgültig erstellten Objekt zu tun hat. Ich kommentiere das natürlich und sagen wir mal, er nimmt es persönlich.

Viel hängt davon ab, wie Sie das kommentiert haben. Vergleichen Sie:

Diese Klasse sieht aus wie etwas zwischen Factory und Container und heißt SomethingBuilder, was nicht wirklich mit dem endgültig erstellten Objekt zusammenhängt.

zu:

Ich frage mich, ob es sich lohnen könnte, dies wie folgt in eine Fabrik und einen Container aufzuteilen:

[.. kleine Menge Pseudocode, um zu demonstrieren, was du meinst ..]

Ich denke, das wird uns [konkreten Vorteil X] verschaffen.

Was denken Sie?

Außerdem scheint es so zu sein, dass Etwas die Absicht nicht wirklich vermittelt, da es eher ein Anderes ist. Vielleicht in Otherthing umbenennen?

Das erste Beispiel, das aus Ihrer Frage paraphrasiert ist, läuft auf kaum mehr hinaus, als sich über Code zu beschweren und zu sagen, dass "Ihr Code scheiße ist". Ich finde es nicht besonders seltsam, solche Kommentare persönlich zu nehmen.

Das zweite Beispiel bietet eine Alternative, formuliert es auch als Vorschlag und bittet um Feedback, anstatt Ihrem Kollegen nur zu sagen, was er tun soll.

Er sagt, jeder hat seinen eigenen Stil. Ich bin mir nicht sicher, was für ein Stil es ist, Factory einen Builder zu nennen, aber egal.

Das klingt einfach abschätzig gegenüber den Ansichten und Konventionen Ihres Kollegen und als ob Sie nicht einmal bereit wären, es ernst zu nehmen. In realen Programmen sind die Dinge oft nicht so ordentlich organisiert; Es mag einen guten Grund geben, es so zu schreiben, wie er es getan hat, aber wenn Ihre Formulierung nicht kommuniziert, dass Sie offen für Alternativen sind, werden Sie dieses Gespräch nicht führen.

Vielleicht schreibt Ihr Kollege nur dummen Code; Ich weiß nicht. Aber ihm zu sagen "du schreibst albernen Code" ist keine sehr effektive Möglichkeit, das zu ändern. Fragen Sie, warum er es so gemacht hat, wie er es getan hat. Hör mal zu. Bieten Sie sinnvolle Alternativen an und erklären Sie, warum Sie es für besser halten. gib ihm Raum, darüber nachzudenken und damit zu experimentieren.

Erwarten Sie nicht, dass er sich ändert, indem er ihm einfach die Wörterbuchdefinitionen von „Fabrik“ oder „Baumeister“ an den Kopf wirft; erklären, wie es die Dinge objektiv verbessern wird. „Eleganter“ wird oft verwendet, ist aber kein sehr gutes Argument. Wenn Sie Schwierigkeiten haben, bessere Argumente als "eleganter" zu finden, dann vielleicht ... ist es nicht so wichtig?

Um ehrlich zu sein, mache ich normalerweise ziemlich gründliche Code-Reviews.

Einige Code-Reviews können "zu gründlich" sein und Dinge kommentieren, die nicht sehr wichtig sind.

Bei meinen eigenen Code-Reviews halte ich immer inne und überlege, bevor ich einen Kommentar hinterlasse: „Wird das die Code-Qualität wirklich objektiv verbessern, oder ist das nur meine persönliche Vorliebe?“ Nicht selten ist es nur meine persönliche Vorliebe; also hab ich es gelassen.

Ich habe gesehen, wie Leute einige sehr geringfügige subjektive Dinge kommentierten. Es ist in Ordnung, einen kleinen Rechtschreibfehler/Tippfehler in einem Kommentar zu kommentieren, da das objektiv falsch ist, aber ich denke, es ist wichtig anzuerkennen, dass Programmieren ein sehr subjektives Handwerk ist und dass Objektivität schwer zu erreichen ist (deshalb sind Programmierer ständig Streit um alles ).

Mir scheint hier, dass Sie nicht sozusagen der hohe Mann auf dem Totempfahl sind. Daher gibt es wirklich nur eine begrenzte Anzahl von Dingen, die Sie tun können, und es scheint, als hätten Sie sie getan:

1) Wenn das Team keinen PR/CR-Prozess durchführt, sollten Sie darauf drängen, ihn einzuführen (Sie haben es getan).

2) Wenn die Leute PRs absegnen, sollten Sie sie auffordern, damit aufzuhören (das haben Sie getan).

3) Du solltest das Vorbild sein und wenn dir jemand eine schlechte PR schickt, solltest du alles kommentieren und erklären, was falsch ist (das hast du getan).

4) Wenn die Leute nicht auf Hochtouren arbeiten, sollten Sie dies dem technischen Leiter mitteilen (Sie haben es getan).

An diesem Punkt liegt es in der Verantwortung des technischen Leiters, die Situation nach Belieben zu handhaben. In diesem Fall kommt die Kultur von oben; Der erfahrenste Entwickler muss dem Rest des Teams entweder erklären, warum der PR-Prozess wichtig ist, oder ihm auferlegen, dass er es richtig machen muss, auch wenn er nicht versteht, warum (optimalerweise ersteres, aber letzteres funktioniert auch). Wenn der technische Leiter jedoch nicht versteht, warum es sich um einen wertvollen Prozess handelt, oder sich nicht die Mühe machen möchte, sicherzustellen, dass er ordnungsgemäß durchgeführt wird, können Sie nicht viel tun.

Was Sie in Zukunft tun sollten, so schmerzt es mich, dies zu sagen, Sie sollten Probleme ansprechen, aber wenn die Leute auf der Empfängerseite des Problems denken, dass es kein Problem ist, dann sollten Sie sie absegnen. Du läufst Gefahr, vom „Verantwortlichen“ zum „Anal-Zurückhaltenden“ zu werden, und willst nicht den Ruf haben, schwer zu bearbeiten zu sein, das ist es nicht wert. Ich würde auch empfehlen, nach einer neuen Stelle zu suchen; Sie wollen nicht mit schlechten Entwicklern zusammenarbeiten, die Sie mit ihnen zu Fall bringen werden. Lassen Sie sie auf ihrem sinkenden Schiff bleiben, und Sie sollten Ihren verantwortungsvollen Ansatz für die Entwicklung eines Unternehmens anwenden, in dem eine solche Fähigkeit geschätzt wird.

Ich würde hier sagen, bleib bei den Waffen. Ihre Codeüberprüfung klingt vernünftig. Wenn Ihr technischer Leiter schlechte Namenskonventionen zulässt, führt dies zu Verwirrung bei Entwicklern, die diese Klasse nicht geschrieben haben. Namenskonventionen sind für viele Dinge nützlich, und Standard-Namenskonventionen sind der richtige Weg.

Hier sind ein paar praktische Ideen zur Lösung dieses Problems.

1. Versuchen Sie, Code-Review-Kommentare in objektive und subjektive Punkte zu unterteilen. Zu den objektiven Punkten gehören Beobachtungen wie „hier liegt ein potenzieller Fehler vor“ oder „das entspricht nicht den Anforderungen“. Subjektive Kommentare beinhalten "das geht besser" oder "das sollte besser X heißen". Es ist oft hilfreich, den Unterschied in Kommentaren deutlich zu machen: "Meiner Meinung nach ..." / "Erwägen Sie es, es zu versuchen ...". Ich denke, das hilft, die Abwehr zu reduzieren.

2. Akzeptieren Sie, dass Sie nur dann Verbesserungen vorschlagen können, wenn die Punkte subjektiv sind, es sei denn, Sie befinden sich in einer höheren Position. Sie müssen akzeptieren, dass es im Team einige Stilunterschiede geben wird, egal wie offensichtlich es Ihnen scheint, dass ein Weg vorzuziehen ist.

3. Wenn die Diskussion hitzig zu sein scheint, ist es oft hilfreich, sich persönlich zu unterhalten und nicht über das PR-Tool, das Sie verwenden. Sie können eine eingehendere Diskussion führen und mit angemessener Körpersprache taktvoller sein. Sie sind vielleicht immer noch anderer Meinung, aber es könnte einfacher sein, den Standpunkt des anderen zu sehen.

4. Für Dinge wie Namenskonventionen ist es oft nützlich, sich im Team darauf zu einigen, wie Module vom Typ X genannt werden sollen. Ein Slack oder ein anderes internes Kommunikationstool bietet oft eine Umfrageoption für Teammitglieder, um über bevorzugte Namenskonventionen abzustimmen. Sobald Sie eine Teamvereinbarung getroffen haben, können Sie in zukünftigen Codeüberprüfungen darauf hinweisen. Bieten Sie an, eine Wiki-Seite mit den Entscheidungen Ihres Teams einzurichten.

5. In Bezug auf Fragen auf höherer Ebene, ob PRs durchgenickt werden oder wie Konflikte über PRs gelöst werden können, sind dies gute Themen, die Sie in einer Retrospektive ansprechen können (vorausgesetzt, Ihr Team folgt einem agilen Prozess).

Ihr Kollege scheint den gesamten Überprüfungsprozess zu missbilligen. Ihr technischer Leiter ist skeptisch. Dies sind diese beiden Punkte, die Sie ansprechen sollten, beginnend mit demjenigen, der am meisten über den Prozess zu sagen hat.

Der Tech-Lead

Der technische Leiter ist möglicherweise skeptisch gegenüber Ihrer Art, Bewertungen zu erstellen oder überhaupt mit Bewertungen. Sie müssen herausfinden, warum er Ihren Kollegen verteidigt. Sei offen für Kritik. Vielleicht bist du zu förmlich. Vielleicht scheint die Art und Weise, wie Sie Feedback geben, nicht sanft genug zu sein.

Dann sollten Sie ihm verkaufen, dass Überprüfungen jedoch notwendig sind, um die Codequalität und das Teamniveau sicherzustellen. Schlagen Sie vor, dass das Team in Zukunft wachsen könnte und diese Probleme angegangen werden müssen, wenn es weniger schmerzhaft ist.

Seine feste Zustimmung ist der Schlüssel zum Erfolg von Bewertungen. Wenn er nicht einverstanden ist, können Sie den Überprüfungsprozess einfach aufgeben, aber das bedeutet nicht, dass Sie kein Problem mit dem Kollegen haben.

Der Kollege

Entwickler im Allgemeinen (ich definitiv eingeschlossen) neigen dazu, viel Ego in ihre Arbeit zu stecken. Das Problem mit Rezensionen ist nicht neu, manche Leute nehmen es persönlich als Kritik an ihrer Arbeit. Dies ist von entscheidender Bedeutung, da Entwickler teuer und eine gute Moral wichtig sind.

Aus diesem Grund sollten Sie unabhängig davon, ob Ihr technischer Leiter Bewertungen genehmigt, die Tatsache betonen, dass Sie trotz harter Überprüfungen der Meinung sind, dass er gute Arbeit leistet, und er sollte sich nicht unsicher fühlen. Überprüfen Sie, ob er nicht auch in Eile programmiert, erinnern Sie ihn gegebenenfalls daran, dass Sie Qualität statt Quantität bevorzugen.

Ich bin mir nicht sicher, ob es hier viel zu tun gibt. Wenn das ungeheuerlichste Beispiel, das Sie haben, die Benennung einer Klasse ist – und es ist nicht einmal so schlecht benannt, ich bin mir ehrlich gesagt nicht sicher, was der Unterschied zwischen „Erbauer“ und „Fabrik“ ist – dann sollten Sie vielleicht noch einmal überdenken, was Sie ' erneute Überprüfung in einer Codeüberprüfung.

Sehen Sie, eine Überprüfung muss nicht immer ein Problem haben . Wenn der Entwickler den Team-Namenskonventionen entspricht, was auch immer sie sind, dann müssen Sie die Namen nicht kommentieren.

Wenn Sie beispielsweise ein Speicherleck oder einen seltsam ineffektiven Code feststellen, dann ja, beachten Sie das. Aber tappen Sie nicht in die Falle „wenn ich gebeten werde, etwas zu kommentieren, muss ich es auf irgendeine Weise kommentieren“. Du nicht. Sie sind in der Industrie, 75 % sind eine lobenswerte Anstrengung, verdammt, 35 % sind meistens bestanden.

Solange die Dinge funktionieren und keine Fehlfunktionen haben und es rechtzeitig fertig ist, na, was will man mehr? Sie werden nicht durch schöne Codezeilen bezahlt, Sie werden überhaupt nicht durch Codezeilen bezahlt - Sie werden durch praktische Ergebnisse bezahlt .

Es hört sich so an, als würden Sie dem Team viel Arbeit hinzufügen, seien Sie vorsichtig. Betrachten Sie es mal so: Wenn Sie ein Unternehmen leiten würden, würden Sie dann schönen Code wollen, der nichts bewirkt, oder Müllcode, der sich kaum skalieren lässt – aber Sie könnten ihn verkaufen?

Wenn Sie denken, dass es jemanden gibt, der ein Unternehmen führt und das erste will, haben Sie vielleicht Recht - aber sie führen dieses Unternehmen nicht sehr lange.

Eine Diskussion über die Namensgebung

Namenskonventionen sind dumm. Wörter ändern im Laufe der Zeit ihre Bedeutung, und der Versuch, den Namen mit der Funktion zu zementieren, ist sinnlos, wenn die Funktion genau da ist und gelesen werden kann .

Jeder kann den Code lesen, um zu verstehen, was er tut. Aber jeder wird eine andere Vorstellung davon haben, was Wörter bedeuten - daher ist es eine kaputte Prämisse, darauf zu bestehen, dass die Wörter der Bedeutung für Ihr Verständnis der Wörter entsprechen , es sei denn, Sie sind die einzige Person, die jemals den Code lesen wird, was Sie nicht sind.

Schlimmer noch, Sie möchten, dass die Namen mit dem Innenleben übereinstimmen , wenn Namen eigentlich eines von zwei (oder besser beides) tun sollten:

1. Beschreiben Sie die Ausgabe
2. Beschreiben Sie die darin enthaltene Geschäftslogik

Das Innenleben zu beschreiben ist meistens sinnlos, denn auch hier kann der Code gelesen werden. Die Erfassung der Geschäftslogik ist viel, viel wichtiger. Zu Pfingsten:

saveDataInAthreadSafeAndDataConsistentManner(){...}

ist nicht sinnvoll. Aber

saveAddressDetailsAsTheresAConcurrentProblemWhenSavedWithEverythingElse()

Nun, das ist ein nützlicher Methodenname.

Aber am Ende des Tages, funktionierender Code >> gut benannter Code, der nicht funktioniert, und schnell produzierter funktionierender Code >> langsam produzierter gut benannter funktionierender Code.

Ein weiterer Gedanke

Sehen Sie niemals so aus, als wären Sie die Person im Team, die Standards durchsetzt, die das Team ausbremsen – das sieht nicht gut aus. Wenn Ihr Teamleiter Ihnen zustimmt, können Sie dies weiterverfolgen, aber die TL tut dies nicht. Jetzt sieht es also so aus, als würden Sie sich nur um Namenskonventionen kümmern. Ich würde das nicht tun.

Traurig zu sagen, aber Sie haben das Gefühl, dass Sie dieses Team verlassen müssen, vielleicht diese Firma. Sie passen nicht gut in die Kultur. Sowohl Ihr Teamleiter als auch derjenige, der Ihren Teamleiter befördert hat, haben sich entschieden, ein bestimmtes Verhaltensmuster zu belohnen. Die Wahrscheinlichkeit, dass sowohl diese Menschen gehen als auch die gesamte Kultur sich verändert, ist sehr gering.

Anstatt Sie für Ihre Fürsorge zu belohnen, hat Ihr Teamleiter Ihre Versuche, die Arbeitsqualität in Ihrem Team zu verbessern, respektlos und abschätzig behandelt. Dies versetzt Sie in eine politisch schwache Position und ist leicht dafür verantwortlich, dass Sie die Dinge verlangsamen, obwohl der Code tatsächlich „einmal geschrieben und viele Male gelesen“ ist, einfach zu lesen, zu verstehen und zu warten, einen schnelleren Weg zu einem fehlerfreien Produkt bedeutet. Nach Ihrer eigenen Beschreibung hat Sie der Codestil Ihres Kollegen um 30 Minuten verlangsamt, um genau herauszufinden, was nur eine Klasse getan hat. Sicherlich hätte eine solche Beschreibung Teil des Klassennamens sein sollen.

Darüber hinaus hat Ihre Managementkette grundlegende Mathematik und grundlegende Psychologie ignoriert.

Für die Mathematik, wenn ein Modul eine 99-prozentige Chance auf keinen Fehler hat (d. h. nicht genügend Komponententests verwendet), dann können Sie (1-p) ^ n berechnen, dass 100 Module zusammen eine Chance von 37 % auf fehlerfreies Verhalten haben . Die Wahrscheinlichkeiten gehen sehr schnell gegen dich. Eine Verbesserung der „Qualität“ auf 99,5 % verbessert das System auf knapp über 60 %.

Für die Psychologie, wer will schon für oder mit Menschen arbeiten, denen es egal ist?