Der Kollege blockiert die Änderungsanfrage im Peer-Review wegen wahrgenommener Fehler im Code, aber die vorgeschlagenen Verbesserungen funktionieren nicht

In dem Unternehmen, für das ich arbeite, durchlaufen Änderungsanträge verschiedene Schritte, darunter einen Entwicklungsschritt, einen Peer-Review-Schritt und einen Testschritt. Bei dieser speziellen Änderungsanforderung wurde ich als Entwickler und ein Kollege als Prüfer zugewiesen.

Dieser Kollege ist derjenige, der mich ursprünglich ausgebildet hat. Er ist seit über 10 Jahren im Unternehmen, während ich vor einem Jahr, kurz nach dem Studium, einstieg. Er hat mehr Jahre Erfahrung als Entwickler, als ich alt bin. Trotzdem ist er immer noch mein Kollege, da wir beide "Mid Level"-Entwickler (nicht mehr Junior, sondern nicht Senior) sind.

Bei dieser speziellen Änderungsanforderung handelt es sich um eine alte, komplizierte Funktion. Es ist nach jedem Codestandard schlecht geschrieben, aber es funktioniert. Ich brauchte ein paar Stunden, um überhaupt zu finden, was ich ändern musste. Der Auftraggeber wünschte eine andere Auswertungsreihenfolge für die Ermittlung des Initialwerts eines Feldes (z. B. Auftraggeber.Adresse vor Schuldner.Adresse prüfen statt umgekehrt).

Mein Kollege bemerkte, dass ich lange brauchte und schaute mir den Quellcode an. Er wies auf einen Code hin und sagte: "Das müssen Sie löschen". Ich hatte meine Zweifel, aber ich habe es trotzdem versucht. Beim Ausprobieren wurde mir jedoch klar, wo das Problem lag. Am Ende musste ich zwei Aussagen umstellen.

Ich habe die Änderung dokumentiert, getestet und sie funktionierte wie vorgesehen. Ich leitete die Änderungsanfrage an meinen Vorgesetzten weiter, der bestätigte, dass sie funktionierte, und leitete sie an den Prüfer weiter. Ich habe es ein paar Minuten später zurückbekommen mit den Kommentaren "das geht nicht, du hast den falschen Code geändert" und "du hast nicht getan, was ich gesagt habe".

Ich sagte, dass ich versucht habe, was er gesagt hat, aber es hat nicht funktioniert. Der Code, auf den er sich bezog, hatte nichts mit der Änderungsanforderung zu tun, außer dass sie etwas mit demselben Feld machten. Seine Antwort war, dass das, was er sagte, nur ein Hinweis war, weil ich in der Lage sein muss, Dinge selbst herauszufinden.

Jetzt weigert er sich, den Änderungswunsch an die Tester weiterzugeben, bis ich meinen Fehler behebe. Ich habe ihn nach weiteren Details gefragt, und er sagt immer wieder, dass ich es selbst herausfinden muss, weil ich lernen muss. Ihm zufolge muss ich damit beginnen, den Code zu löschen, den er erwähnt. Da dieser Code nichts mit der fraglichen Änderungsanforderung zu tun hat, bin ich sehr zurückhaltend. Ich fragte unseren Manager, und er sagte, ich solle dem Rezensenten meine Argumente vortragen, da der Manager keine Ahnung von Programmierung habe. Ich habe das versucht, aber der Rezensent bleibt bei seinem Standpunkt und entlässt mich mit "tu einfach, was ich gesagt habe".

Teilweise aufgrund des Alters dieses Codes gibt es keine Komponententests und sie können nicht für sie geschrieben werden, ohne die halbe Codebasis neu zu schreiben.

Wie behebe ich diese Situation?

Nur aus Neugier, ist dies eine Unternehmens-IT-Abteilung oder ein IT-Entwicklungsshop?
Können Sie den Änderungswunsch ohne seine Hilfe an die Tester weitergeben?
Code von Kollegen überprüfen zu lassen, ist Zeitverschwendung. Wenn der Kodex funktioniert, fällt alles, was falsch sein könnte, eher in die Kategorie des Urteils und der Meinung als in die Kategorie der objektiven Tatsachen, und das macht es zu einer Aufgabe des Managements.

Antworten (9)

Er mag Recht haben, dass der Code repariert werden muss, aber er geht bei jeder Codeüberprüfung, die ich je gesehen habe, falsch vor. Codeüberprüfungen sollten für sehr spezifische, adressierbare Punkte fehlschlagen, z. B.:

  1. Inkonsistente Einrückung in ScoreHandler.cpp, Zeilen 45-50. Verstößt gegen Stilrichtlinie Nr. 8.
  2. getConsistentReview()in Foo.java ist nicht wie erforderlich Thread-sicher und synchronisiert auf der falschen Sperre.
  3. getFoo()in Foo.java iteriert über eine Liste, um Ergebnisse herauszufiltern. Da unsere Quellebene jetzt 8 ist, sollte dies stattdessen die Stream-API zum Filtern verwenden, was viel prägnanter ist.

Es hört sich so an, als hätten Sie jemanden, der (schlecht) versucht, Sie zu betreuen, indem er Sie dazu bringt, Code auf eine von ihm gewünschte Weise zu "reparieren", ohne Ihnen genau zu sagen, warum er kaputt ist oder was daran falsch ist. Es ist nicht einmal klar, ob der Grund, warum er möchte, dass Sie es ändern, stilistischer oder funktionaler Natur ist.

Sie könnten sich einfach weigern, Ball zu spielen, und es ihm zurückschieben. Ob Sie glauben, dass das insgesamt funktioniert oder ob es mehr Probleme verursacht, hängt von der Kultur ab.

Wenn Sie eine sanftere Herangehensweise wünschen, versuchen Sie, genauer zu beschreiben, was Sie versucht haben, warum das nicht funktioniert hat, und geben Sie genau an, was Sie tun müssen, um fortzufahren:

Hallo x,

Ich habe zuvor versucht, yCode zu löschen, wie Sie vorgeschlagen haben, aber das führt stattdessen zdazu, dass die Funktion (diese) Tests in (diesen) Fällen nicht besteht. Ich bin mir in Bezug auf die Anforderung hier sehr unklar - ist die Änderung, die Sie anfordern, eine stilistische oder eine funktionale? Wenn es stilistisch ist, könnten Sie auf die spezifischen Stilrichtlinien hinweisen, gegen die der Code verstößt, und wenn es funktional ist, könnten Sie mir einen Testfall zur Verfügung stellen, der fehlschlägt?

Keines der oben genannten Dinge ist unvernünftigerweise in einer Codeüberprüfung zu verlangen - es ist unvernünftiger, dass es nicht von Anfang an bereitgestellt wurde.

Wenn er danach immer noch sagt "mach einfach, was ich versucht habe", kannst du einfach zurückdrängen mit "Wie oben, ich fürchte, das hat nicht funktioniert und ich brauche viel mehr Details, um das Problem zu lösen." Wenn jemand Sie wegen zu langer Zeit anprangert, zeigen Sie ihm die Papierspur; Es sollte ziemlich schnell klar werden, dass er derjenige ist, der seine Füße schleppt und sich weigert, Ball zu spielen.

Update: gerade ausprobiert und es hat funktioniert. Am Ende kam er zu meinem Schreibtisch und half mir, meinen Code zu verbessern. Es war schließlich nicht falsch, aber könnte besser sein. Seine Lösung war nicht vollständig, aber nicht so falsch, wie ich dachte. Ich schätze, wir haben uns beide geirrt.
@Belle-Sophie Vielleicht hat er diesen Beitrag gesehen und festgestellt, dass Sie wirklich feststecken.
@Belle-Sophie Schön zu hören, helfe gerne.
@Belle-Sophie: Ich denke, es ist eine gute Lektion für die Zukunft; Ja, Code-Reviews werden im Allgemeinen asynchron über Tools durchgeführt. Wenn Sie sich jedoch durch eine Frage / Antwort im Code-Review verwirrt fühlen, ist es möglicherweise besser, auf Telefon- / Face-to-Face-Kommunikation umzusteigen, um Missverständnisse schnell zu klären :)
Hatten seine Änderungen dennoch einen Vorteil? Oder war es nur Zeitverschwendung für alle?
Eine andere Möglichkeit besteht darin, dass der Prüfer zusätzliche Anwendungsfälle gesehen hat, die nicht behoben wurden. Dies können zusätzliche Fehler sein oder auch nicht, abhängig von den Details des Fehlerberichts und davon, wie Fehler verwaltet werden. Wie auch immer, der Ansatz funktioniert immer noch.
@ Belle-Sophie: Bitte aktualisieren Sie Ihre Frage damit, da dies die gesamte Prämisse Ihrer Frage ziemlich ungültig macht
OPL "Der Code, auf den er sich bezog, hatte nichts mit der Änderungsanforderung zu tun, außer dass sie etwas mit demselben Feld gemacht haben ." [Hervorhebung von mir]. Es hängt also zusammen!

Im Workflow meiner Firma wäre das einfach.

Markieren Sie die Änderungsanforderung als „wird nicht behoben“ mit einem Kommentar „korrekte Korrektur vom Prüfer abgelehnt“ und weisen Sie ihm dann die Änderungsanforderung zu. Offensichtlich weiß er, wie man es repariert, also sollte das ein Fünf-Minuten-Job für ihn sein.

Und eigentlich, wenn er Recht hätte, dann wäre es das Richtige. Es hat keinen Sinn, Ihre Zeit zu verschwenden. Er kann sich also über nichts beschweren.

Bei altem und obskurem Code können zwei Dinge passieren: Ich lasse ihn unverändert, oder ich verbessere ihn und übernehme die Verantwortung, wenn er kaputt geht. Das Einzige, was nicht passieren wird, ist, dass ich jemand anderem befehle, diese Änderungen vorzunehmen. Und schon gar nicht während eines Code-Reviews.

Habe das versucht. Er wirft es mir einfach zurück.
Das ist, wo Sie es zu Ihrem Manager bringen. Ihr Manager hat keine Ahnung von Programmierung, aber Sie sagen, die Vorschläge Ihrer Kollegen funktionieren nicht, er sagt, sie funktionieren, also wäre es in dieser Situation für Ihren Chef offensichtlich, dass er die Arbeit machen sollte.
@Belle-Sophie Also... besteht er darauf, dass du seine Änderung unter deinem Namen vornimmst? Das ist nicht sehr Cricket. Ich schlage vor, dieses Detail zu Ihrer Frage hinzuzufügen, da es die Mentalität des Typen unterstreicht
@rath Kannst du deine Verwendung des Wortes „Cricket“ in diesem Zusammenhang erklären? Ich fürchte, ich bin völlig verloren.
@Two-BitAlchemist ist ein englischer Begriff, der "einfach nicht die richtige Art, Dinge mit Ehre anzugehen" bedeutet, von Cricket-Spielen, bei denen von den Spielern immer erwartet wurde, mit Fairness und Gentleman-Verhalten zu spielen.
Ja. Weißt du, was nicht "Cricket" ist? Auf Douchebaggery mit noch mehr Douchebaggery antworten. Passiv-aggressive Kommentare im Code (und Code-Reviews) sind ein großes Warnsignal für die Eignung einer Person zur Zusammenarbeit. Tu es nicht.
„Wird nicht behoben“ ist normalerweise ein enger Grund auf einem Ticket. Warum würden Sie das Ticket schließen, bevor Sie es neu zuweisen?
Wenn Sie also Probleme weitergeben, wissen Sie nicht, wie Sie es tun sollen; wie willst du jemals lernen, wie man sie macht?

Dies trifft aus technischen oder organisatorischen Gründen möglicherweise nicht auf Sie zu, aber die allgemeine Antwort lautet:

Machen Sie einen Unit-Test

Ein Unit-Test ist der Beweis, dass ein Fehler behoben wurde. Jedes Mal, wenn Sie einen Fehler beheben oder wenn Sie einen Fehler demonstrieren müssen, sparen Komponententests eine Menge Argumente ein. Sehen Sie nach, ob es eine QA-Person gibt, mit der Sie sprechen können, und klären Sie das Problem.

Denken Sie daran, dass er Ihre Änderung nicht abgelehnt hat, weil sie nicht funktioniert hat, er hat sie abgelehnt, weil es nicht seine Änderung war. Ein Unit-Test im Rücken zu haben, stärkt Ihre Position.


Speziell für Ihre Situation: Denken Sie daran, dass er es selbst tun sollte, wenn er möchte, dass Sie tun, was er gesagt hat. Es ist Ihre Aufgabe und Sie können tun, was Sie sagen. Das ist Ihre Mentalität, jetzt zum Dialog.

Rufen Sie den Entwickler an und sprechen Sie mit ihm. Zeigen Sie, dass Ihr Fix funktioniert, und wenn Sie gefragt werden, warum Sie nicht getan haben, was er gesagt hat, können Sie so etwas sagen:

Das ist nicht genau das, was Sie gesagt haben, aber es funktioniert und behebt das Problem. Ich verstehe diese Lösung in meinem Kopf, der andere Weg würde wahrscheinlich auch funktionieren, aber ich hätte viel mehr Zeit gebraucht, um es herauszufinden. Ich denke also, wir können diesen in der Codeüberprüfung als Bestanden markieren ?

Ich mag diese Antwort. Leider funktioniert es bei mir nicht (ich habe die Frage aktualisiert), aber es ist eine gute Antwort für Leute mit einem ähnlichen Problem, die tatsächlich testbar ist.
@Belle-Sophie Ich dachte, deine Situation könnte etwas anders sein :) Der Typ klingt wie ein Kollege, den ich nicht haben möchte
Es hört sich so an, als hätten Sie eine Lösung, aber für die nächste Person in dieser Situation – während es normalerweise unpraktisch ist, eine klobige Legacy-Funktion vollständig zu testen, ist es fast immer möglich, jedes einzelne Stück Logik zu einer testbaren Funktion herauszurechnen und sie dann zu verknüpfen zusammen, und oft führt dies dazu, dass Ideen entstehen, wo der Fehler liegt.

Ich stimme berry120 zu. Ihre Bewertungsnotizen waren es jedoch

„Das geht nicht, du hast den falschen Code geändert“ und „Du hast nicht getan, was ich gesagt habe“

Mein Vorschlag:

(Verbringen Sie nicht mehr als einen halben Tag oder so damit. Dokumentieren Sie relevantes Codeverhalten, Fehler / Warnungen, Ihre Maßnahmen und Gründe dafür präzise.)

  • Stellen Sie sicher, dass Ihr neuer Code in Version X (keine Fehler oder Warnungen) gemäß der Kundenanforderung funktioniert
  • Erstellen Sie vor Ihren Änderungen einen Fork oder eine Version des Codes
  • tun Sie genau das, was in den Notizen angegeben ist (versuchen Sie, Fehler zu beheben, falls vorhanden), dies ist jetzt Version Y
  • Wenn diese Änderungen die Clientanforderung nicht erfüllen, vermerken Sie dies besonders und versuchen Sie, Version X in Version Y aufzunehmen
  • Informieren (per E-Mail) Ihren Bewerter und Manager:

Ich glaube, ich habe die Implementierung der vom Kunden angeforderten Änderungen in Version X abgeschlossen.
Um die Überprüfungshinweise zu berücksichtigen, habe ich die folgenden zusätzlichen Schritte für Version Y unternommen.

Listen Sie hier Ihre Fortschritte und Ergebnisse auf, fügen Sie Ihre Dokumentation dieses Prozesses bei (fassen Sie sich kurz, max. 5-10 Zeilen).
Wenn es der Lesbarkeit dient, können Sie diese Liste auch am Ende der E-Mail haben und entsprechend darauf verweisen.

Wenn die Überprüfungsnotizen nicht auf die Kundenanfrage eingegangen sind:
(Im Folgenden wird Ihre Verzögerung und anfängliche Nichteinhaltung der Notizen erklärt)

Soweit ich das beurteilen konnte (siehe meinen Bericht oben/unten), würden die Überprüfungshinweise allein nicht auf die Kundenanfrage eingehen, und ich habe zusätzlich Version X implementiert.

Ich habe zuerst Version X geschrieben, um sicherzustellen, dass die Kundenanfrage berücksichtigt wurde, bevor andere Codeänderungen hinzugefügt wurden.

Sollte Version Y immer noch Fehler enthalten, die Sie nicht beheben konnten:
(Wahrscheinlich werden es die Review-Notizen oder die Ergänzung der Review-Notizen um Version X sein – Wortlaut unten entsprechend anpassen!)

Ich freue mich, weiterhin an Version Y zu arbeiten.
Leider erweist sich das Integrieren der Überprüfungsnotizen in Version X (der funktionierende Client hat Änderungen angefordert) als komplexer als das Einreichen von Version X allein, und ich benötige mehr zugewiesene Zeit.

Ich bin kurz davor, das Folgende einzufügen, um Ihren Rezensenten etwas zu besänftigen und gleichzeitig das Management darauf hinzuweisen, dass seine Änderungen die Fehler verursachen. Es kann auch dazu führen, dass das Management es so interpretiert, dass Sie immer noch die Aufsicht des Gutachters benötigen.

Geben Sie den Namen des Prüfers ein, wenn Sie einen Moment Zeit haben, würde ich mich sehr über Ihre Anleitung bezüglich der Motivation hinter (oder möglicherweise weniger neugierig: "der Art") Ihrer Codeänderungen und der daraus resultierenden Fehler, die ich für Version Y erhalte, freuen .

Verwenden Sie offensichtlich die tatsächlichen Versionsnummern anstelle von X / Y (;

BEARBEITEN:

Um meine Antwort lesbarer und prägnanter zu machen, habe ich sie geändert. Bitte hinterlassen Sie Kommentare, wenn dies besser ist. Vielen Dank.

Mir ist etwas Ähnliches passiert - bei meinem ersten Job nach dem College musste ein leitender Entwickler eine Änderung vornehmen, um etwas zu unterstützen, an dem ich arbeitete, aber die Änderung, die er vornahm, funktionierte nicht. Was am Ende für mich funktioniert hat, und was ich Ihnen vorschlage, ist Folgendes:

Beziehen Sie einen anderen Senior Developer mit ein, planen Sie ein Treffen mit beiden und erläutern Sie Ihren Standpunkt. Demonstrieren Sie nach Möglichkeit die von Ihnen durchgeführten Tests. Dann lassen Sie ihn seine Vorgehensweise erklären. Finden Sie heraus, was die beste Lösung ist, und tun Sie das.

Stellen Sie sicher, dass Sie nicht als Gegner wirken, etwa so wie:

Hallo Mike. Es scheint, als würden wir ständig aneinander vorbeireden über Bug #123. Haben Sie am Montag 30 Minuten Zeit, um darüber nachzudenken? Ich werde Jim zu uns holen, da er in letzter Zeit daran gearbeitet hat

Bei meiner letzten Stelle hatten wir einen Peer-Review-Prozess. Uns wurde vom Management gesagt, wir sollten der Person nicht sagen, wie sie das Problem lösen soll, sondern ihnen nur Hinweise und Anleitungen geben. Als solcher war ich am anderen Ende der OP-Situation, wo ich der leitende Entwickler bin, der eine neue Person schult, die ein Problem "behoben" hat, aber die Änderungen berücksichtigen bestimmte einzigartige Situationen nicht.

Die Art und Weise, wie ich es gelöst habe, bestand darin, die einzigartigen Situationen direkt zu erklären, und die Person konnte dieses Verhalten codieren, aber der Gesamtcode war fehleranfällig, da es sich um ein Legacy-Repo handelte. Mein Gedanke ist, dass Sie den Entwickler fragen sollten, ob es eine einzigartige Situation gibt, von der Sie nicht wissen, dass er sie vorhersehen kann. Wenn es keine Einzelfälle gibt und er sagt, dass es "kaputt" ist, weil Sie ihm nicht gefolgt sind, dann legen Sie es ihm wieder an. Schreiben Sie in das Ticket, dass der Code funktioniert, er wurde vom Management getestet, und leider sehen Sie keine Situation, in der der Code falsch wäre.

Es klingt seltsam, dass er Ihre Lösung ablehnt und sich weigert, Ihnen Einzelheiten mitzuteilen. Es ist also an der Zeit, ein wenig zu eskalieren. Das bedeutet, dass Sie die E-Mail, die er Ihnen gesendet hat, an Ihren Teamleiter senden, in der steht, dass Sie nicht verstehen, warum Ihr Fix abgelehnt wurde, und dass Sie die Funktionalität nicht genug verstehen, um herauszufinden, was Ihr Kollege sagt, also bitten Sie den TL um Hilfe.

Ich würde dies als "alte Funktionalität ausdrücken, deren Geschichte oder Auswirkungen Sie nicht verstehen", aber Sie möchten das vielleicht verfeinern, aber Sie gehen zum TL, "um Hilfe bei dieser Aufgabe zu erhalten", in dem Wissen, dass er es tun wird Sehen Sie sich die Spur der Codeänderungen an (z. B. "Zeigen Sie mir, was Sie bisher versucht haben"), die die mangelnde Kooperation Ihres Kollegen hervorhebt. Das sollte ausreichen, um diese Aufgabe im Team sichtbar zu machen und sich darauf zu konzentrieren, sie zu beheben, ohne künstliche und wenig hilfreiche Hindernisse durch Ihren Kollegen (der sich unprofessionell verhält).

An keiner Stelle brauchen Sie sich zu beschweren oder gar die mangelnde Hilfsbereitschaft des Kollegen zu erwähnen, das ist klar.

Wenn Sie keinen Teamleiter haben, setzen Sie es wieder in die „Todo“-Warteschlange und erwähnen Sie (in einem Fortschrittsmeeting?), dass Sie das Problem nicht ausreichend verstehen, um es über das hinaus zu beheben, was Sie bereits versucht haben Überlasse die Reparatur jemandem mit mehr Erfahrung.

Ihr Kollege hat Ihnen einige Hinweise gegeben und erklärt, dass Sie lernen müssen. Sie versuchen, hilfreich zu sein und Ihnen die Tools an die Hand zu geben, mit denen Sie ähnliche Probleme in Zukunft selbst beheben können.

Sie haben ihre Hinweise ausprobiert, aber aus einem unbekannten Grund haben sie nicht funktioniert (entweder waren die Hinweise falsch, Sie haben sie nicht verstanden oder eine Kombination aus beidem). Sie können keines dieser Probleme alleine lösen, daher müssen Sie sich erneut an den Prüfer wenden, um Hilfe zu erhalten. Wenn Sie das tun, sollten Sie dem Prüfer erklären, was Sie versucht haben und warum es nicht funktioniert hat.

Dies zeigt, dass Sie sich selbst anstrengen, anstatt sich auf ihre Erfahrung zu verlassen, um das Problem zu beheben (was Sie zu einem „Hilfsvampir“ machen könnte). Wenn ihre Hinweise falsch waren, werden sie es an dieser Stelle merken und Sie können dann andere Lösungen besprechen. Wenn Sie ihre Hinweise falsch verstanden haben, werden sie Ihr Denken besser verstehen und ihre Hinweise umformulieren können, um Sie von Ihrem Missverständnis abzubringen.

Wenn Sie die Kommentare lesen, besteht die Antwort, die Sie tatsächlich zu verwenden schienen, darin, mit der Person über Ihre Schwierigkeiten und ihre Erwartungen zu sprechen. und dann herausfinden, warum das, was sie erwartet hatten, nicht funktionierte.

Unabhängig davon, welche Tools Ihr Unternehmen zur Verfolgung von Problemen verwendet, handelt es sich in der Regel um Tools zur Aufzeichnung und Kommunikation. und obwohl sie ihren Platz haben, denke ich, zeigt dies, dass verbale Kommunikation nicht ersetzt werden kann.

Um ehrlich zu sein, sollte dies die erste Anlaufstelle für viele arbeitsbezogene Probleme sein; denn es gehört einfach dazu, wie man in einem Team arbeitet.