Kollege überprüft Code zu spät im Prozess

Im Laufe des Sommers wechselte ich von einem großen Unternehmen als leitender Ingenieur zu einem viel kleineren Unternehmen als leitender Ingenieur. Jetzt beaufsichtige ich etwa 20 Ingenieure auf mittlerer bis mittlerer Ebene, die an 3 verschiedenen Reservierungs- und Buchungssystemen arbeiten. Alle Buchungssysteme laufen auf demselben proprietären Back-End-Framework, das von einem anderen Team verwaltet wird, das ich nicht beaufsichtige.

Letzten Monat, in der Nacht vor dem Start eines großen Releases für eines der Buchungssysteme, hinterließ ein leitender Ingenieur („Clint“) aus dem Back-End-Supportteam eine Reihe von Kommentaren zu einem Pull-Request, den wir geöffnet hatten, um unseren Release Candidate darin zusammenzuführenMaster

Das Feedback reichte von etwas hilfreich bis etwas nicht hilfreich. Wir fusionierten Mastertrotzdem zu und am nächsten Tag fragte ich ihn, ob er diesen Code früher hätte überprüfen können, anstatt bis nach dem Integrationstest zu warten. Als er das Feedback hinterließ, war es das Ende des Tages und wir bereiteten ein Ticket für unseren Release Engineer vor, das am nächsten Morgen um 4:30 Uhr bereitgestellt werden sollte.

Er sagte mir, dass es nicht seine Aufgabe sei, meine Ingenieure zu unterrichten (er hat recht, das ist es nicht). Aber es fällt mir schwer, 20 Ingenieure gleichzeitig zu coachen, selbst wenn sie Peer-Reviews durchführen und den Code der anderen überwachen. Ich mache mir auch Sorgen, dass mein Team etwas demotiviert war, da sie nichts tun konnten, um auf das Feedback einzugehen.

Wir haben eine weitere Veröffentlichung geplant, gleich nachdem wir von Thanksgiving zurück sind, und basierend darauf, wie Clint's alle unsere Einladungen zu Code-Review-Meetings in diesem Monat abgelehnt hat, denke ich, dass ich in ein paar Tagen eine Wiederholung derselben Sache sehen werde.

Ich kann nicht sagen, ob Clint wirklich helfen oder nur sein Ego spielen lassen will. Ich würde seine Hilfe beim Coaching unserer Junior-Entwickler lieben, aber die Art und Weise, wie er es tut, ist nicht hilfreich. Ich glaube nicht, dass meine Ingenieure jemals in der Lage sein werden, alles zu erfassen, was Clint kann.

Wie kann ich Clint sagen, wenn er Feedback geben möchte, muss es zu unseren Bedingungen sein?

EDIT : Es ist mir peinlich, dass ich dieses Detail ausgelassen habe, aber unsere Ingenieure öffnen Pull-Requests aus ihren Feature-Zweigen, developmentwohin dieses Feedback gehen sollte (zu diesen Pull-Requests) ... wenn diese Änderungen alle bereit sind, in unsere Produktionsumgebung zu gehen ( Nachdem unsere QA-Ingenieure Integrationstests durchgeführt und überprüft haben, dass die Änderungen ihrer Meinung nach sicher sind), öffnen wir einen PR, um ihn zusammenzuführen, Masternachdem eng die Änderungen genehmigt und zugestimmt haben, dass wir nichts Schreckliches eingeführt haben, und stellen sie dann am nächsten Morgen bereit

Welche Rolle spielt Clint im PR/Merge-Prozess? Benötigen Sie seine Bewertung, um zusammenführen zu dürfen? Berät er nur?
Wie ist hier der Gesamtprozess? Nach dem, was Sie geschrieben haben, wurden Integrationstests durchgeführt, und Sie haben eine PR, die für die Zusammenführung mit dem Master offen ist, und Sie implizieren auch, dass Sie als Ergebnis vom Master bereitstellen. Warum wird der PR nach dem Integrationstest durchgeführt? Wie lange war die PR geöffnet? Welche anderen Möglichkeiten für Feedback gab es? Was passiert mit Ihren Tests, wenn ein PR-Kommentar gemacht wird, das ein grundlegendes Problem hervorhebt? Es hört sich so an, als gäbe es hier in einem breiteren Kontext mit dem gesamten Team viel zu besprechen, und nicht unbedingt ein Problem damit, dass Clint Kommentare zur PR hinterlässt, imho.
Warum haben Sie mit der Fusion bis zum letztmöglichen Moment gewartet? Gibt es einen Grund, warum Sie nicht ein paar Stunden früher zusammenführen wollten, zu diesem Zeitpunkt wäre wahrscheinlich genug Zeit gewesen, um auf Kommentare einzugehen? Wenn ein Pull-Request offen ist, wird erwartet, dass die Leute Kommentare hinterlassen – das scheint eher ein Problem auf Ihrer Seite als auf ihrer Seite zu sein. Außerdem, woher sollte er wissen, dass er nichts kommentieren sollte? Beachten Sie, dass Sie, wenn die Kommentare selbst verlangen, dass Sie im Grunde Zeit damit verschwenden, Dinge unnötig zu ändern, eine andere Frage haben.
Es ist eine Sache von OP zu klären, aber für alle Kommentatoren, die vorschlagen, dass die PR früher hätte gemacht werden sollen, wenn ihr Prozess in etwa so ist, wo ich arbeite (Git-Flow), dann ist die PR/Zusammenführung zum Master, wie die Bereitstellung in der Produktion ist Ausgelöst. Es hätte viele frühere PRs zu einem Entwicklungs- (oder Stamm-) Zweig für jedes einzelne Feature und PRs zu verschiedenen Release-Zweigen für QA oder andere Umgebungen gegeben. Code-Reviews hätten dann stattfinden sollen, nicht bei der endgültigen Zusammenführung zum Master.
@sleske Gute Frage! Das mag die Ursache unseres Problems sein, aber ich sehe seine Rolle darin, uns wissen zu lassen, dass keine unserer Änderungen schädlich ist, basierend auf dem, was er von der Buchungsplattform versteht. Ich kann ohne seine Überprüfung zusammenführen, und er gibt nur Ratschläge
@Moo Vielen Dank für die durchdachte Antwort, ich hoffe, ich kann den Gesamtprozess gut genug beschreiben: QA-Ingenieure führen Feature-Zweige einen nach dem anderen mit unserem Entwicklungszweig zusammen, dann stellen wir den Entwicklungszweig für Integrationstests in unserer Entwicklungsumgebung bereit. Wenn die Tests bestanden werden, führen sie die Entwicklung bis zum Master für Rauchtests in einer Staging-Umgebung zusammen. Änderungen werden erst nach bestandenen Integrationstests und per Pull-Request als zusätzliche Möglichkeit, Breaking Changes abzufangen, zum Master zusammengeführt. Ich hätte gerne sein Feedback früher, wenn wir richtig reagieren können, aber er will es dann nicht geben
@Dukeling Wir versuchen, den Master-Zweig mit der Produktion synchron zu halten und kurz nach der Zusammenführung damit bereitzustellen. Unsere QA-Ingenieure erwarten einen Tag zum Testen der Änderungen im Entwicklungszweig, bevor sie kurz darauf mit Master für Rauchtests zusammengeführt und für die Produktion bereitgestellt werden. Meine Frage bezieht sich darauf, Feedback auf Dinge zu beschränken, die eine Bereitstellung blockieren würden, wenn die Person, die Feedback anbietet, diese Art von Einschränkung nicht möchte
@DavidConrad Ja, nah genug. Es ist nicht automatisch, aber wir versuchen, neue Commits von Master fernzuhalten, bis sie gründlich getestet wurden und wir für die Bereitstellung in der Produktion bereit sind
Codeüberprüfung seine Codeüberprüfung? Code Review Review: „Interessantes Feedback. Stellenweise falsch. Benötigt einen proaktiveren Ansatz. Üben Sie besseres Timing.“
Sie erwähnen nicht, dass Sie Clint jemals darüber informiert haben, wann Sie erwarten, dass er das Feedback abschließt. Haben Sie ihm überhaupt einen Zeitplan mitgeteilt? Wenn Sie Zeitpläne haben, mit denen er arbeiten muss, muss er das wissen, damit er seine Aufgaben und Prioritäten seinen Vorgesetzten mitteilt.

Antworten (10)

Sofern Clint keine größeren „Do not deploy“-Fehler findet, würde ich ihm einfach für sein Feedback danken und ihm erklären, wie Sie die von ihm angesprochenen Punkte (falls zutreffend) in zukünftigen Versionen angehen wollen.

Wenn er damit ein Problem hat, dann haben Sie die Möglichkeit zu erklären, warum es für ihn besser wäre, früher Feedback zu geben.

Letztlich ist es sein Problem, dass er so spät Feedback gibt. Machen Sie es nicht zu Ihrem eigenen, indem Sie das Feedback als persönliches/Team-Versagen ansehen.

+1 das ist die richtige Antwort. Feedback ist Feedback, aber wie die Antwort sagt, wird das Feedback später behandelt, es sei denn, "Clint" hat ein Blockerproblem gefunden. Wenn "Clint" damit nicht zufrieden ist, können sie den Code früher überprüfen.
Würden Sie immer noch sagen, dass es sein Problem ist, wenn er nicht wüsste, dass sie an diesem Abend fusionieren wollten?
Es ist nicht sein Problem, wenn dies seine erste wirkliche Gelegenheit war, Feedback zu geben. Und selbst wenn es so wäre, sein Problem ist Ihr Problem ist das Problem aller, weil Sie alle Teil desselben Teams sind und alle wollen, dass es erfolgreich ist, oder?
Ich verstehe nicht, warum das Clints Problem ist. Clint ist Teil eines anderen Teams, es scheint, dass er unabhängig vom Prozess Feedback gibt. Das Problem ist, dass OP es nicht als sehr nützlich ansieht (aufgrund des Timings). Ich denke, niemand hat dort eine klare Vorstellung davon, worum es beim Code-Review gehen sollte, oder jeder hat seine eigene Idee. Es kann vielen Zwecken dienen. Sie sollten auf die gleiche Seite kommen.
Ich denke, diese Antwort ist richtig - ich sehe hier einfach kein "Problem". Vielleicht verstehe ich das falsch? Clint lieferte neue Informationen, von denen OP einige für wertvoll hält. Genial. Das ist immer gut so. Wenn diese Informationen darauf hindeuten, dass Sie nicht bereitstellen oder zusammenführen können oder was auch immer, dann ist es BESSER, dies so schnell wie möglich zu wissen. Wenn diese Informationen auf geringfügige Probleme hinweisen, großartig, protokollieren Sie sie und sortieren Sie sie entsprechend. Keine Probleme hier. Wenn Sie besser auf Clints Feedback reagieren möchten, müssen Sie ihn natürlich früher einbeziehen, aber ich sehe dies immer noch als eine vollkommen gute Situation, so wie sie ist.
So sollte ein Unternehmen damit umgehen. Es ist ein Fehler zu glauben, dass jeder Code-Review-Kommentar eine Aktion erfordert. Sobald Sie offensichtliche Fehler entdeckt haben (hoffentlich von der ersten Person, die dies überprüft), werden viele Kommentare „Dinge, die nett wären“ oder „Dinge, die Sie beim nächsten Mal besser machen sollten“ oder einfach nur „Dinge, die Sie anders machen könnten, aber vielleicht werde ich so weitermachen".
Lol, es ist Clints Problem in dem Sinne, dass WENN er möchte, dass sein Feedback ernst genommen wird, DANN es an Clint liegt, dieses Feedback früh genug im Prozess zu geben.
Kommentar zum Tagesabschluss für einen Einsatz um 4:30 Uhr am nächsten Tag? Verdammt, ich habe die Verfahren geändert, um Bereitstellungen nur von Montag bis Mittwoch zuzulassen, weil jemand eine Bereitstellung am Freitag nach dem Arbeitstag vermasselt und am Montag einen riesigen Berg an Arbeit verursacht hat. Clint hätte nicht erwarten sollen, dass das Feedback umsetzbar wäre, abgesehen von den schwerwiegendsten Show-Stopper-Fehlern.
Hier wird nicht angesprochen, wie dieses Problem mit dem Team besprochen werden kann. Aber es ist ziemlich offensichtlich – Sie erklären dem Team, dass dies ein sehr hilfreiches Feedback ist, das es ihnen ermöglicht, es beim nächsten Mal besser zu machen. Kein bereitgestellter Code ist jemals perfekt und es gibt immer Dinge, die Sie besser machen können, und jemand, der Sie darauf hinweist, hilft Ihnen beim Lernen.
Ich möchte auch hinzufügen, dass dies auch eine Managementfrage ist. Alle Codeüberprüfungen sollten vor der QA durchgeführt werden. Code-Fixes vor der QA sind billig, Code-Fixes nach der QA sind teuer, weil sie die QA erneut bestehen müssen; es wird nur repariert, wenn es ein Showstopper ist. Wenn jemand nach der Qualitätssicherung eine Codeüberprüfung durchführt, verschwendet das das Geld des Unternehmens. Entweder sollte der Prozess so adressiert werden, dass die Codeüberprüfung rechtzeitig erfolgt, oder Sie kümmern sich überhaupt nicht darum.
@RobP. Das Problem, das ich sehe, ist, dass OP Clints Feedback zu ernst nimmt. Es ist eindeutig nicht beabsichtigt, sich auf die Veröffentlichung zu beziehen, da Clint wahrscheinlich kein Idiot ist, und sollte daher für zukünftige Veröffentlichungen berücksichtigt werden.

Entweder ist für die Freigabe eine Codeüberprüfung erforderlich, oder sie ist nicht erforderlich. Ist dies erforderlich, so hat der Gutachter für die rechtzeitige Begutachtung zu sorgen. Sie müssen die Macht haben, zu seinem Schreibtisch zu gehen und zu sagen: "Lassen Sie alles andere fallen und machen Sie diese Überprüfung, oder wir können das Veröffentlichungsdatum nicht einhalten." Und Sie müssen die Möglichkeit haben, zu sagen: "Entschuldigung, wir konnten nicht rechtzeitig veröffentlichen, weil die Überprüfung nicht rechtzeitig fertig war".

Wenn es nicht erforderlich ist, geben Sie es frei.

PS. Wenn Clint einen Tag gegeben wird, um Wochen oder Monate der Arbeit zu überprüfen, scheint dies darauf hinzudeuten, dass die Überprüfung nicht erforderlich war. Aber wenn Clint in dieser kurzen Zeit Probleme findet, scheint das darauf hinzudeuten, dass frühere Rezensionen nicht ganz so gut waren, wie sie hätten sein sollen.

+1 - Verantwortung ohne die Macht, Dinge wirklich zu tun, ist ein Problem, das ich öfter gesehen habe, als ich möchte.
Angesichts der Tatsache, dass „Clint“ anscheinend weniger als einen Tag Zeit hatte, um Mannwochen oder vielleicht Mannmonate der Arbeit zu überprüfen, scheint diese Antwort ziemlich daneben zu liegen. Es ist eindeutig unvernünftig, den Prüfer für die schnellere Überprüfung des Codes „verantwortlich“ zu machen, als es ihm tatsächlich möglich ist, ihn zu überprüfen. Und da Clint es anscheinend geschafft hat , in den wenigen Stunden, die er hatte, einige nützliche Kommentare zu hinterlassen, scheint es umso perverser zu behaupten, dass irgendetwas behoben werden würde, wenn nur das OP die Möglichkeit hätte, Clint zu zwingen, seine eigenen Prioritäten fallen zu lassen und zu überprüfen oder zu beschuldige Clint, zu langsam zu sein.
@MarkAmery Ich sehe in der Frage keinen Hinweis darauf, dass Clints Zeit durch OP begrenzt war. Wenn OP die Macht hätte zu fragen: "Wie viel Zeit benötigen Sie für die Überprüfung?" und dann die Macht zu sagen "OK, also müssen Sie am oder vor dem Datum X beginnen, weil wir vor Y Feedback brauchen", dann wäre es in Ordnung. Wenn OP dies nicht tun kann, sollte er nicht dafür verantwortlich gemacht werden, dass er nicht auf Feedback reagiert. Es sollte ein Managementproblem sein, kein OP. Es geht nicht darum, Clint die Schuld zu geben, sondern darum, OP und sein Team nicht dazu zu zwingen, das Unmögliche zu tun.
+1 Außerdem steht es dem Team von OP frei, vor der nächsten Veröffentlichung auf Feedback von Nicht-Blockern einzugehen.
@Mołot Peter Parkers Onkel Ben war nah dran, als er sagte: "Aus großer Kraft folgt große Verantwortung". Sie kommen nicht zusammen. Sie sind dasselbe. Wenn ich nicht die Macht habe, zu kontrollieren, wie etwas gemacht wird, bin ich nicht dafür verantwortlich, wie es falsch gemacht wird. Zu viele Menschen versuchen, Macht und Verantwortung voneinander zu trennen, aber es geht einfach nicht, und die Versuche, dies zu tun, scheitern. Immer wenn jemand versucht, mich verantwortlich zu machen, wo ich keine Macht habe, sage ich: "Was genau hätte ich Ihrer Meinung nach anders machen können, um ${BAD_THING} zu verhindern?"; Die Antwort lautet normalerweise "nichts".
@MontyHarder Den Begriff "verantwortlich" neu zu definieren, um ihn der aktuellen Diskussion anzupassen, ist kaum förderlich. Wenn sie Sie nach der Bewertung fragen und Ihre Antwort erwarten und Sie antworten (hauptsächlich, weil Sie die Antwort intern mit Ihrem beruflichen Selbstverständnis verknüpfen), dann ist es das: Sie sind so verantwortlich wie möglich für die Sache. Sie können eine solche Definition anpassen, ohne die vollständige Macht oder Kontrolle über jeden Aspekt zu haben.
@Mołot Meine Lektüre der Abfolge der Ereignisse war, dass weniger als 24 Stunden zwischen dem Öffnen des PR (und damit für Clint zur Überprüfung verfügbar) und dem Zusammenführen über seine Einwände vergingen. Diese Interpretation wird nun vom OP bestätigt. Es wurde irgendwann während eines Arbeitstages geöffnet, Clint schaffte es, eine Überprüfung einzubauen, und dann ignorierte das Team alles und verschmolz am nächsten Tag um 4:30 Uhr, wobei das OP Clint beschuldigte, die PR, die dies tat , nicht früher überprüft zu haben existieren noch nicht . Es scheint mir ziemlich verrückt zu sein.
@MarkAmery unter solchen Umständen ist es ein Problem mit dem Zeitplan. Und ich denke immer noch, dass OP entweder in der Lage sein sollte, zu fragen, wie viel Zeit Clint benötigt und entsprechend einzuplanen, oder, wenn er keine Erfolgsmacht hat, OP nicht für die kurze Zeit verantwortlich gemacht werden sollte, die Clint hatte, oder die Überprüfung der Fakten wurde ignoriert. Die einzige Person, die schuld ist, ist derjenige, der einen so zu engen Zeitplan entworfen hat.
@Mołot Sicher, es liegt möglicherweise nicht in den Händen des OP. (Oder vielleicht nicht. Sie gehen, glaube ich, davon aus, dass der Zeitplan außerhalb der Kontrolle des OP lag, aber nichts bestätigt dies in der Frage; das OP verwaltet ein großes Team und legt seinen Zeitplan plausibel fest oder beeinflusst ihn zumindest.) Unabhängig davon löst keine Antwort ein, die die Situation speziell und ausschließlich auf die mangelnde Macht des OP über Clint zurückführt, wenn das OP zugibt, dass Clint am selben Tag, an dem ihm etwas zur Überprüfung gegeben wurde, sofort nützliches Feedback hinterlassen hat. Keine noch so große Macht wird es dem OP ermöglichen, Clint dazu zu zwingen, Code zu überprüfen, bevor er existiert.
@MarkAmery Laut der Frage wurde Clint zu Code-Reviews eingeladen, hat aber nicht akzeptiert. Es gibt keine Beweise dafür, dass OP den Code bis spät im Prozess zurückhält. Clint entscheidet sich anscheinend selbst dafür, erst am Ende des Prozesses eine Überprüfung vorzunehmen. Die Schlussfolgerung ist, dass Clint nicht glaubt, dass der Input, den er bekommen könnte, seine Zeit nicht wirklich wert ist.
@DavidThornley Ja, laut der Frage wurde Clint jetzt zu Code-Review-Meetings eingeladen. Selbst wenn wir seine Ablehnung als Beweis dafür nehmen, dass er sich entscheidet , sie erst am Ende des Prozesses zu überprüfen (im Gegensatz zu beispielsweise, dass sie mit wichtigeren Besprechungen kollidieren oder er von seinem Vorgesetzten angewiesen wird, keine Zeit mit der Arbeit eines anderen Teams zu verbringen für sie), hat das nichts mit dem vom OP beschriebenen vorherigen Vorfall zu tun. Die Schlussfolgerung folgt überhaupt nicht, da wir keine Informationen darüber haben, warum Clint die Treffen abgelehnt hat.

Es hört sich so an, als gäbe es hier eine Reihe von Problemen, aber alle sind ansprechbar.

Prozessprobleme:

  • Was ist der Zweck dieses Pull-Requests? Soll es überprüft werden, oder ist es einfach ein Mittel, um von einem QA-unterstützten und getesteten Zweig zum Master zu führen? Wenn ersteres der Fall ist, sollten Sie eine andere Zeitplanung für den Überprüfungsprozess in Betracht ziehen. Im letzteren Fall sollte es fast sofort nach seiner Erstellung zusammengeführt werden.
  • Was machen Sie mit "verspäteten" Bewertungskommentaren? Es hört sich so an, als hätten Clints Kommentare nicht den vollständigen Überprüfungsprozess durchlaufen, obwohl sie „zu spät“ waren. (Was definiert „verspätet“?) Selbst wenn Sie sich zur Zusammenführung verpflichtet haben und keiner der Kommentare ein Showstopper ist, sollte es möglich sein, seine Kommentare als umsetzbar oder nicht einzustufen und in der PR entsprechend zu antworten.

Persönliche Probleme:

  • Ich bekomme hier ein bisschen eine „Wir sind nicht alle ein Team“-Stimmung. Das mag aus verständlicher Frustration Ihrerseits herrühren, aber wenn Sie Clint respektieren – und ich nehme an, dass Sie das tun –, ist es angebracht, zu versuchen, dies zu beheben. Möglicherweise lehnt er jetzt Ihre Besprechungsanfragen ab, weil er beschäftigt ist, oder er hat das Gefühl, dass Sie seinen Beitrag sowieso nicht wollen.
  • Hinterfüllung, um Ihre wahren Absichten zu zeigen. Wenn Sie denken, dass einige der Dinge, die er gesagt hat, echte Probleme waren, reichen Sie Tickets ein und stellen Sie sicher, dass er darin enthalten ist.

Wenn Sie die PR bis zum Abend vor dem Push für eine "echte Überprüfung" offen lassen, bedeutet dies, dass Sie entweder alle Bewertungen ernst nehmen und nicht zusammenführen und nicht pushen müssen, wenn es nicht adressierte Kommentare gibt, oder Sie müssen die Push-Planung so ändern, dass es welche gibt genügend Zeit, um die Überprüfung abzuschließen und den Push zu planen oder abzubrechen (z. B. werden PRs, die nicht mindestens 24 Stunden vor dem geplanten Push zusammengeführt wurden, nicht versendet).

Es könnte eine gute Idee sein, ihn auf einen Kaffee einzuladen und darüber zu sprechen, indem Sie ihn wissen lassen, dass Sie seinen Beitrag zu schätzen wissen, aber dass der Prozess so, wie er jetzt ist, bedeutet, dass Sie seine Bewertung nicht früh genug erhalten haben Handeln Sie diesmal danach und betonen Sie "dieses Mal". Lassen Sie ihn wissen, dass Sie den Prozess ändern möchten, und fragen Sie ihn, ob er Vorschläge hat.

Fragen Sie ihn, was es ihm erleichtern würde, einen Beitrag zu leisten, und stellen Sie sicher, dass er versteht, dass Sie es zu schätzen wissen, dass er sich die Mühe gemacht hat. Wenn er wirklich versucht zu helfen, dann demotiviert ihn auch keine Reaktion auf seine Kommentare. Es hört sich so an, als hätte es keine Stopper gegeben, seit Sie mit der Zusammenführung fortgefahren sind, aber es klingt auch so, als wäre die Arbeit aus seiner Sicht vergebliche Mühe gewesen.

Danken Sie ihm , dass er die Probleme gefunden hat, auf die er hingewiesen hat, und lassen Sie ihn wissen, dass Sie beabsichtigen, die wichtigen Dinge anzusprechen, die er angesprochen hat. Sie sollten in Betracht ziehen, dies sowohl persönlich mit ihm als auch öffentlich in der geschlossenen PR zu tun, um ihm und dem Team klar zu machen, dass Sie froh sind, dass er hilft. Sich privat und öffentlich für seine Fürsorge und freiwillige Hilfe zu bedanken, selbst wenn keiner der Bewertungskommentare aus der Sicht Ihres Teams umsetzbar war, ist nur höflich und hilft, Solidarität zwischen den Teams aufzubauen.

Genauer gesagt könnte Clint das Gefühl haben, in ein Team und eine Arbeitsbelastung hineingezogen zu werden, die nicht in seiner Verantwortung liegt, wenn er bereits genug (möglicherweise sogar zu viel!) Eigenes hat, um das er sich Sorgen machen muss. In diesem Fall kann es der Beziehung entgegenwirken, ihn in Fehlerberichte einzubeziehen und andere Anstrengungen zu unternehmen, damit er sich beteiligter / geschätzter fühlt.
IME-Supporttechniker sind oft überlastet. Es wäre hilfreich, Clints Gründe für die Ablehnung der Code-Review-Einladungen herauszufinden (vielleicht ist er einfach zu beschäftigt). Clint wissen zu lassen, dass seine Beiträge wertvoll sind, wenn auch spät, wäre eine gute Möglichkeit, ihn dem OP-Team näher zu bringen. Wenn Clint Kapazitäten hat, aber das Gefühl hat, nicht nah genug zu sein, um sich früher einzumischen, fühlt er sich vielleicht einfach unwohl dabei, anderen Leuten auf die Füße zu treten.
Ja, deshalb gehst du Kaffee trinken und redest darüber. Dieses eine Mal scheint er sich freiwillig gemeldet zu haben, also scheint es der direkteste Weg zu sein, sich zu melden und zu sehen, ob und wie er etwas beitragen möchte, und in einer informellen Umgebung kann er Dampf ablassen, wenn er Lust dazu hat wurde nicht geschätzt.

Wie kann ich Clint sagen, wenn er Feedback geben möchte, muss es zu unseren Bedingungen sein?

Sofern Clint nicht für Sie arbeitet oder es einen formellen Prozess gibt, der vorschreibt, wann Kommentare nicht erlaubt sind, können Sie Clint nicht zwingen, sich an „Ihre Bedingungen“ zu halten.

Seine Kommentare kannst du ignorieren. Sie können ihm für die "etwas hilfreichen" Kommentare danken, um mehr davon zu ermutigen. Sie können Elemente zu Ihrem technischen Rückstand hinzufügen, um auf diese hilfreichen Kommentare einzugehen. Sie können ihn weiterhin zu Code-Review-Meetings einladen. Sie können Ihr Team ermutigen, sich nicht von ein paar verspäteten Kommentaren demotivieren zu lassen.

1) Gibt es niemanden zwischen Ihnen und 20 Ingenieuren, der Ihnen helfen kann, sie zu betreuen? Aus diesem Grund sind 20 Personen zu viele Personen in einem Team, da eine Person auf keinen Fall 20 Personen verwalten und betreuen kann. Sie müssen die Größe Ihres Teams reduzieren oder zumindest die Größe Ihrer Verantwortung, weil Sie viel zu dünn gestreut sind.

2) In Bezug auf die Veröffentlichung des Codes, ist Clint in Ihrem Team? Wenn Clint in Ihrem Team ist, dann sollte Clint an Code-Reviews beteiligt sein, besonders wenn er ein leitender Ingenieur ist. Sie sollten Ihre Mentees dazu ermutigen, wenn möglich Code-Reviews an Clint zu senden (überladen Sie ihn nicht, aber ermutigen Sie sie, Clint in den Prozess einzubeziehen). Wenn Clint nicht in Ihrem Team ist, lassen Sie ihn die Probleme als Fehlerberichte protokollieren, es sei denn, die Probleme, die Clint findet, sind kritische, betriebsunterbrechende Fehler. Er ist nicht in Ihrem Team, er ist nicht für Ihr Produkt verantwortlich.

4) Was Clints „Code-Review-Meetings“ vermisst: Erstens bin ich mir nicht sicher, was ein „Code-Review-Meeting“ ist. Code-Reviews sind asynchrone Vorgänge: Der Entwickler reicht den Code ein, der Code-Reviewer prüft, während der Entwickler etwas anderes macht, Review beendet, Entwickler spricht Kommentare an, spülen, wiederholen. Ich weiß nicht, was ein "Code-Review-Meeting" ist, es klingt albern. Aber abgesehen davon, wenn Clint in Ihrem Team ist, sollte Clint an Teambesprechungen teilnehmen. Wenn Clint nicht in Ihrem Team ist, muss Clint nicht an Teambesprechungen teilnehmen. So einfach ist das.

"Code-Review-Meetings" - Das haben wir früher gemacht. Einmal pro Woche ging einer der leitenden Entwickler verschiedene aktuelle Merge-Anfragen durch und wies auf verschiedene Beispiele für besonders guten und schlechten Code hin und insbesondere darauf, was daran gut oder schlecht war. Dann hätten alle eine große Diskussion darüber. Es ist eine großartige Möglichkeit, Erfahrungswissen in etwas zu destillieren, das Junioren verstehen und aufnehmen können.
Codeüberprüfungen können asynchron sein. Es können auch Gruppentreffen sein, bei denen der Code durchgegangen wird. Es stellt sich heraus, dass es produktiver sein kann, eine Gruppe zu haben, da verschiedene Leute unterschiedliche Dinge fangen.

Jemand muss den "Prozess" definieren, zB die Reihenfolge, in der Dinge passieren.

Wenn ich als Entwickler nicht derjenige bin, der den Prozess definiert, mache ich eine Codeüberprüfung (wenn das meine Aufgabe ist, wie von meinem Manager erwartet), wenn der Prozess es mir sagt und/oder wenn mich jemand darum bittet.

Ich verstehe den Teil in der Frage nicht, in dem es heißt: "Alle unsere Einladungen zu Code-Review-Meetings in diesem Monat abgelehnt".

Ich bin mir übrigens nicht sicher, was ein "Code-Review-Meeting" ist. Meine Vorstellung von einem Code-Review ist:

  1. Jemand codiert es
  2. Ich überprüfe es (in meiner eigenen Zeit) und mache mir Notizen
  3. Ich treffe mich mit dem Programmierer, um meine Bewertung zu besprechen

Wenn ich "Senior" bin, höre ich vielleicht nicht auf die Besprechungen anderer Leute.

Um meine Zeit zu sparen (meinen Aufwand zu reduzieren), überprüfe ich gerne Code, nachdem er getestet wurde. Ich könnte immer noch Fehler finden (z. B. wenn das Testen nicht perfekt abgeschlossen ist), aber es ist einfacher, Code zu überprüfen, der funktioniert, als Code, der nicht funktioniert. Das Überprüfen von Code, der nicht funktioniert, wird als "Entwicklung und Debugging" bezeichnet und ist zeitaufwändiger.

Können Sie "Integrationstests" einfacher machen, vielleicht automatisierter? Damit Sie tun könnten:

  • Entwicklung
  • Integrationstest
  • Überprüfung
  • Rezensionskommentare korrigieren
  • Nochmal Integrationstest

Alternativ können Sie vor der Codeüberprüfung zusammenführen (wenn Sie darauf vertrauen, dass die Integrationstests ohne Überprüfung ausreichend waren), die Codeüberprüfung im Hauptzweig durchführen und die Codeüberprüfungskommentare als Eingabe dafür verwenden, was Sie bei einer nachfolgenden Iteration verbessern möchten (eine nachfolgende "Sprint").

-1 für "Codeüberprüfung sollte nach dem Testen erfolgen". Einige Tests können automatisiert werden, aber viele Tests sind manuell (dies gilt insbesondere für Frontend-Code, kann aber auch für Backend-Code gelten). Manuelles Testen kann lange dauern; Code-Reviews sind im Allgemeinen kurz.
Mit "nach dem Testen" meine ich mindestens nach dem erneuten Ausführen automatischer Rauchtests sowie alles, was Sie tun, um zu testen, ob die neue Funktion wie angegeben funktioniert. Verschiedene Systeme haben einen unterschiedlichen Grad an Automatisierung in ihren Tests (und ich sagte, wenn ihre Tests stärker automatisiert wären, könnten sie es sich leisten, dies häufiger zu tun).
Das Testen während der Entwicklung unterscheidet sich vom Testen in Vorbereitung auf die Veröffentlichung und kann je nach Test sehr viel Zeit in Anspruch nehmen, sodass Sie die Häufigkeit Ihrer Ausführung einschränken sollten.
@JoeW Ja und "die Nacht vor einer großen Veröffentlichung" klingt ziemlich [zu] spät. Ich schätze, dass "das Feedback von etwas hilfreich bis etwas nicht hilfreich" bedeutet, dass er keine Show-Stopper gefunden hat, und daher war es richtig, dass das OP pünktlich veröffentlicht wurde, als sie es taten. Und das ist nicht alles schlecht – die Tatsache, dass ein Code-Review keine Showstopper findet, ist nicht unbedingt eine schlechte Sache. Was ist seine Aufgabe, wenn nicht „meine Ingenieure zu unterrichten“? Soll man sich die Änderungen im Integrationszweig vor der Veröffentlichung nur ansehen (dh rauchen)?
@ChrisW Ich stimme definitiv zu, dass ungetesteter Code nicht überprüft werden sollte (ich würde noch einen Schritt weiter gehen und sagen, dass er überhaupt nicht festgeschrieben werden sollte!). Wie Joe jedoch sagte, unterscheiden sich die Tests, die ein Entwickler gegen seinen Code ausführen sollte, von den Tests, die eine QA durchführen würde. Ersteres, klar. Letzteres eher nicht.
Ich denke, Sie haben beim Testen ein kritisches Problem übersehen, bevor Sie den Code selbst überprüft haben. Wenn die Integrations-/Smoke-Tests eine beträchtliche Zeit in Anspruch nehmen, ist es dann sinnvoll, die Tests auszuführen und dann eine Peer-Review durchzuführen, die Probleme finden könnte, die eine erneute Ausführung der Tests erfordern? Wäre es nicht sinnvoller, die Überprüfung vorab durchzuführen, damit potenzielle Probleme vor dem Testen gefunden werden können, damit Sie die Häufigkeit dieser Tests minimieren können?
@JoeW Ich nehme an, Sie haben Recht, dh das OP implizierte, dass seine "Integrationstests" kostspielig oder zeitaufwändig waren - also sollte der "Prozess" vielleicht angeben, dass die Inspektion vor dem Integrationstest durchgeführt wird? Wenn es vor dem Integrationstest ist, ist das vor oder nach der Integration (ich weiß es nicht)? Vielleicht habe ich versucht, Clive zu entschuldigen, dh zu erraten, welche Motive Clive haben könnte. Wenn er so spät noch einmal rezensieren möchte, dann sollte seine Inspektion vielleicht als Teil des Integrationstests betrachtet werden, also einfach "go/no-go", und all seine halbwegs hilfreichen Stilkommentare verworfen werden ...
... oder gesichtet, z. B. könnte das OP sie optional zu den Code-Review-Richtlinien seines Teams hinzufügen oder sie für das Code-Review-Training verwenden.
"Das Überprüfen von Code, der nicht funktioniert, wird als 'Entwicklung und Debugging' bezeichnet und ist zeitaufwändiger." Ich denke, das ist eher ein Kompromiss, und es könnte von den Fähigkeiten/Erfahrungen der Entwickler abhängen. Die Überprüfung vor QA-Tests kann insgesamt sparen, wenn Sie größere Probleme finden, die eine weitgehende Überarbeitung der Arbeit bedeuten; Es macht nicht viel Sinn, Code zu testen, der weitgehend verworfen werden muss (z. B. finden Sie heraus, dass ein Code, der einige tausend Berichtsergebnisse abruft, verwandte Daten Datensatz für Datensatz erfasst). Ich versuche, nicht nach tatsächlichen Fehlern zu suchen, sondern mich stattdessen auf Dinge wie den allgemeinen Ansatz und die Normen zu konzentrieren.

Ich bin mir ziemlich sicher, dass dieser Typ dir nur sagt, dass du einen schlechten Job machst. Das bedeutet "es ist nicht meine Aufgabe, Ihre Ingenieure zu unterrichten". Er ist nicht daran interessiert, Ihre Arbeit für Sie zu erledigen, er möchte, dass Sie es besser machen. Aus diesem Grund hat er Code überprüft, der für die Produktion bestimmt ist (Code, den Sie abgemeldet haben), nicht Code in der Pipeline. Was man dagegen tun kann, nun, das ist ein Thema für eine andere Frage.

Ich glaube ehrlich gesagt, das trifft den Nagel auf den Kopf. Die meisten Leute, die nicht in Ihrem Team sind, wollen keine zusätzliche Arbeit von Ihrem Team. Sie wollen nur, dass Ihr Team gute Arbeit leistet, damit sie nicht mehr darüber nachdenken müssen.
Ich stimme zu, dass dies ein wahrscheinliches Szenario ist. Da „Clint“ länger an der Codebasis gearbeitet hat, kennt er sie wahrscheinlich auch besser.

In acht nehmen.

✓ Leitender Entwickler
✓ Findet Probleme im Code, die niemand sonst gemacht hat
✓ Team aufgrund der verspäteten Meldung entmutigt
✓ Berichtet etwas über „nicht seine Aufgabe“, wird aber trotzdem überprüft
✗ bereitet sich auf eine Wiederholung des gleichen Szenarios vor

Es sieht nicht so gut aus für das Team und auch nicht für den Senior Developer.

Aber ich sage dir, pass auf. Stellen Sie sicher, dass Sie alle Kommentare zu dieser Pull-Rezension verstehen . Und ich meine wirklich verstehen. "Das ist einfach falsch." zählt nicht, es sei denn, Sie haben sich wirklich die Mühe gemacht, dies zu überprüfen.

Wahrscheinlich gibt es nichts Wichtiges, das darauf wartet, für Sie zerstörerisch zu sein. Aber es könnte einen äußerst subtilen Datenverlustfehler geben, den er gefunden hat und der beim Testen nicht aufgedeckt werden kann. Sie werden den Unterschied nicht erkennen, bis Sie alle Kommentare verstanden haben.

Nachtrag: Um die Teammoral wiederherzustellen, stellen Sie sicher, dass Sie genügend Zeit haben, um alle Probleme zu beheben, die der Sr. Developer in dieser Pull-Anforderung gefunden hat, die der Rest des Teams beheben sollte.

Das Problem hier ist, dass Ihr Workflow-Prozess fehlerhaft ist. Peer-Reviews sollten vor dem Integrationstest stattfinden, da das Warten bis nach dem Testen den Prozess verlangsamen kann. Wenn die Peer-Review Probleme findet, die Änderungen erfordern, und Integrationstests bereits durchgeführt wurden, wird mehr Zeit benötigt, um zurückzugehen und alle Tests zu wiederholen, nachdem die Änderungen abgeschlossen sind.

Idealerweise sollte der Entwickler Tests schreiben und durchführen, während er den Code entwickelt, und alle abschließenden Tests sollten durchgeführt werden, nachdem der Code einem Peer-Review unterzogen wurde, um sicherzustellen, dass er wie erwartet funktioniert. Eine Sache, an die man sich erinnern sollte, ist, dass ein Peer-Review Fehler im Code erkennen kann, bevor sie ein System während des Integrationstests herunterfahren können.

"Das Problem hier ist, dass Ihr Workflow-Prozess fehlerhaft ist." Ich verstehe diese Perspektive nicht. Mir scheint, dass das Team versucht, frühzeitig Peer-Reviews durchzuführen, und das Problem ist, dass Clint sie bis zur letzten Minute ignoriert. Der Workflow-Prozess scheint in Ordnung zu sein, aber Clint scheint sich zu weigern, effektiv teilzunehmen.
@DarthFennec Warum möchten Sie jemals Integrationstests durchführen, bevor Sie eine Peer-Review des Codes durchführen? Für mich scheint das fehlerhaft zu sein, da Sie alle durchgeführten Tests wiederholen müssen, wenn Probleme mit dem Code in der Überprüfung gefunden werden.
Dem widerspreche ich nicht. Ich sage, OP scheint auch nicht anderer Meinung zu sein. "Am nächsten Tag fragte ich ihn, ob er diesen Code früher hätte überprüfen können, anstatt bis nach dem Integrationstest zu warten." Es hört sich so an, als ob OP und sein Team versuchen, die Peer-Review vor dem Integrationstest durchzuführen, und das angegebene Problem ist, dass der Prüfer (Clint) gegen diesen Workflow-Prozess handelt.
@DarthFennec Ein Teil des Problems hier ist der Mangel an Details der ursprünglichen Frage, wodurch wir sehr unterschiedliche zugrunde liegende Erzählungen in die Geschichte hineinlesen können. Ich denke, Sie lesen, dass das Team wollte, dass Clint es überprüft, aber er hat sich bis zur letzten Minute vor seinen Pflichten gedrückt. Ich habe gelesen, dass Clint nicht an dem Projekt beteiligt war und keine Autorität darüber hatte, dann am Tag der Deadline mit einer riesigen PR von Mannmonaten von Mistcode überfallen wurde, den er vorher nicht überprüfen konnte, und dann Das OP ärgerte sich über das "späte" Feedback und fügte es über Clints Einwände zusammen.
@MarkAmery "wurde dann am Tag der Frist überfallen" scheint das nicht dem Verhalten von OP zu widersprechen? Warum würde OP sagen "Ich habe gefragt, ob er diesen Code früher hätte überprüfen können, anstatt bis nach dem Integrationstest zu warten", wenn Clint nach dem Integrationstest mit dem Code überfallen wurde? Warum sollte Clints Antwort „das ist nicht mein Job“ statt „Ich wusste es vor dem Integrationstest nicht“ lauten? Ich sage nicht, dass Sie falsch liegen, ich verstehe nur nicht, wie Sie zu Ihrer Schlussfolgerung gekommen sind. Ich stimme jedoch zu, dass es besser gewesen wäre, wenn OP dies klarer ausgedrückt hätte.
@DarthFennec Ich stimme zu, dass die Haltung des OP angesichts meiner Interpretation irrational erscheint, aber sie wird durch die Bearbeitung des OP bestätigt. Die Abfolge der Ereignisse ist: 1. Eine Reihe von Entwicklungen, an denen Clint keine Rolle spielt, werden vom OP-Team durchgeführt, 2. Release-PR wird geöffnet, 3. Clint wird zur Gesundheitsprüfung gebracht und führt dies am selben Tag durch und geht einige negative Rückmeldungen, 4. die Veröffentlichung erfolgt sowieso um 4:30 Uhr am nächsten Tag, 5. OP beschwert sich bei Clint über die "verspätete" Rückmeldung. Der Kommentar "Das ist nicht mein Job" scheint vernünftig von jemandem zu sein, der über die Pflicht hinausgegangen ist, umfangreiches Feedback für Mitglieder von ...
@DarthFennec ... ein anderes Entwicklungsteam und wurde dann dafür beschimpft und gesagt, dass sie mehr und früher an einem Projekt hätten arbeiten sollen, das von Anfang an nicht ihnen gehörte. Die Reaktion des OP darauf scheint nur eine unbedeutende Schuldzuweisung ohne große Begründung zu sein – was zumindest etwas Menschliches und Verständliches ist.
@MarkAmery Aus der Bearbeitung: "Wir öffnen eine PR, um sie mit dem Master zusammenzuführen, nachdem der Eng die Änderungen genehmigt hat." Ich habe dies so interpretiert, dass Clint einer dieser Engs war, und er wartete mit der Genehmigung der Änderungen, bis die PR an den Master vorgenommen wurde , anstatt es vor oder parallel zu den Integrationstests der QA zu tun, wie er es sollte. Aber es könnte wirklich in beide Richtungen gehen. Die Tatsache, dass OP mehrmals gefragt wurde, was Clints Rolle sei, und das in der Bearbeitung nicht direkt beantwortet hat, ist verdächtig; Wenn er der Frage absichtlich ausweicht, verleiht das der Interpretation der Schuldzuweisung Glaubwürdigkeit.
@MarkAmery Es gibt keinen Hinweis darauf, dass Clints Eingabe erforderlich war. (In der Tat wurde die Bereitstellung vorgenommen, bevor seine Kommentare berücksichtigt werden konnten.) OP lädt Clint zu Code-Reviews ein und befürchtet, dass die Kommentare in letzter Minute, die für diese Veröffentlichung möglicherweise nicht nützlich sein können, erneut vorkommen werden. OP möchte offensichtlich, dass Clint hilft, sein Team zu schulen, und offensichtlich will Clint das nicht tun. Jeder Vorwurf der Schuldzuweisung sollte wirklich zeigen, dass Schuld überhaupt existiert, und das sehe ich nicht.

Sie haben zwei verschiedene Arten von Bewertungen (drei, wenn Sie Ihr persönliches Treffen mitzählen). Das ist an sich nicht schlecht, aber es bedeutet, dass Sie Ihre Erwartungen für jeden Umstand sehr deutlich machen müssen. Ich würde eine Pull-Request-Vorlage für Ihre abschließende Überprüfung erstellen, die in etwa so aussieht. Dies macht nicht nur deutlich, was von dieser Bewertung erwartet wird, sondern auch, wie Sie Ihr Feedback in Zukunft erfolgreicher bearbeiten können.

<Zusammenfassung der Änderungen>

Dies ist eine Überprüfung vor der Produktion. Sein Zweck ist es, große Probleme zu finden, wie:

  • Fehler zusammenführen
  • Versehentlich ein unvollständiges Feature enthalten
  • Showstopper-Bugs

Abgesehen von solchen Problemen wird diese Pull-Anforderung am <Datum und Uhrzeit> zusammengeführt.

Die einzelnen Design-Reviews, in denen wir Lesbarkeit, Wartbarkeit und Gesamtarchitektur besprechen, sind bereits abgeschlossen. Wenn Sie diese Art von Problemen hier finden, öffnen Sie bitte ein Problem oder nehmen Sie die Änderungen in Ihrer eigenen Pull-Anforderung an die Entwicklung vor, anstatt diese Pull-Anforderung zu überladen. Die Design-Reviews wurden in den folgenden Pull-Requests durchgeführt:

  • <Link zu PR 1>
  • <Link zu PR 2>
Kollege überprüft Code zu spät im Prozess
AntwortenHierDatenschutz-Bestimmungen1y, 0v