Un collègue examine le code trop tard dans le processus

Joe Stevens

2018-11-20 10:49:49 UTC

view on stackexchange narkive permalink

À moins que Clint ne trouve de bogues majeurs, «ne pas déployer», je le remercierais simplement pour ses commentaires et lui expliquerais comment vous comptez aborder les points qu'il soulève (s'ils sont valides) dans les prochaines versions.

S'il a un problème avec cela, alors vous avez la possibilité d'expliquer pourquoi il serait préférable pour lui de donner son avis plus tôt.

En fin de compte, c'est son problème il donne des commentaires si tard. Ne vous en faites pas en considérant les commentaires comme un échec personnel / d'équipe.

+1 c'est la bonne réponse.Les commentaires sont des commentaires, mais comme la réponse l'indique, à moins que "Clint" ne trouve une sorte de problème de blocage, les commentaires seront traités plus tard.Si "Clint" n'est pas satisfait de cela, ils peuvent revoir le code plus tôt.

Diriez-vous toujours que c'est son problème s'il ne savait pas qu'ils voulaient fusionner ce soir-là?

Ce n'est pas son problème si c'était sa première véritable opportunité de donner son avis.Et même si c'était le cas, son problème est que votre problème est le problème de tout le monde parce que vous faites tous partie de la même équipe et que vous souhaitez tous qu'elle réussisse, non?

Je ne vois pas comment c'est le problème de Clint.Clint fait partie d'une autre équipe, il semble qu'il donne des commentaires indépendamment du processus.Le problème est que OP ne le perçoit pas comme très utile (à cause du timing).Je pense que personne là-bas n'a une idée claire de ce que devrait être la révision du code, ou chacun a sa propre idée.Cela peut servir à plusieurs fins.Ils devraient être sur la même longueur d'onde.

Je pense que cette réponse est juste - je ne vois tout simplement aucun «problème» ici.Peut-être que je ne comprends pas?Clint a fourni de nouvelles informations, dont certaines, selon OP, sont précieuses.Impressionnant.C'est toujours une bonne chose.Si cette information indique que vous ne pouvez pas déployer ou fusionner ou quoi que ce soit, alors il vaut mieux le savoir dès que possible.Si cette information indique des problèmes mineurs, génial, enregistrez-la et triez-la de manière appropriée.Aucun problème ici.Si vous voulez être plus réactif aux commentaires de Clint, alors bien sûr, vous devez l'impliquer plus tôt, mais je vois toujours cela comme une situation parfaitement correcte en l'état.

C'est ainsi qu'une entreprise doit le gérer.C'est une erreur de penser que chaque commentaire de révision de code nécessite une action.Une fois que vous avez détecté des erreurs évidentes (avec un peu de chance par la première personne à examiner), de nombreux commentaires seront "des choses qu'il serait bien de faire" ou "des choses à faire mieux la prochaine fois" ou simplement "des choses que vous pourriez faire différemment maiscontinuera peut-être à faire de même ».

Lol, c'est le problème de Clint en ce sens que * SI * il veut que ses commentaires soient pris au sérieux, * ALORS * c'est à Clint de fournir ces commentaires suffisamment tôt dans le processus.

Commentaire de fin de journée pour un déploiement à 4h30 le lendemain?Heck, j'ai changé les procédures pour autoriser les déploiements uniquement du lundi au mercredi parce que quelqu'un a f / ké sur un déploiement vendredi après la journée de travail et a créé une énorme montagne de travail le lundi.Clint n'aurait pas dû s'attendre à ce que les commentaires soient exploitables, sauf pour les bogues de type show-stop les plus graves.

Cela ne traite pas de la manière de discuter de ce problème avec l'équipe.Mais c'est assez évident - vous expliquez à l'équipe que ce sont des commentaires très utiles qui leur permettront de faire mieux la prochaine fois.Aucun code déployé n'est jamais parfait et il y a toujours des choses que vous pouvez faire mieux et quelqu'un qui vous le fait remarquer vous aide à apprendre.

J'ajouterais également que c'est aussi un problème de gestion.Toutes les révisions de code doivent être effectuées avant le contrôle qualité.Les correctifs de code pré-QA sont bon marché, les correctifs de code post-QA sont coûteux car ils doivent à nouveau passer le contrôle qualité;ça ne se corrige que si c'est un frein à l'émission.Si quelqu'un effectue un contrôle qualité après révision du code, cela gaspille l'argent des entreprises.Soit le processus doit être abordé de manière à ce que la révision du code ait lieu à temps, soit ne dérange pas du tout.

@RobP.Le problème que je vois est qu'OP prend trop au sérieux les commentaires de Clint.Il n'est clairement pas destiné à s'appliquer à la sortie en cours, car Clint est peu susceptible d'être un idiot, et doit donc être gardé à l'esprit pour les versions futures.

gnasher729

2018-11-20 17:40:16 UTC

view on stackexchange narkive permalink

Soit une révision du code est requise pour la version, soit elle n'est pas obligatoire. Si cela est nécessaire, le réviseur doit être responsable de l'examiner à temps. Vous devez avoir le pouvoir d'aller à son bureau et de dire "laissez tomber tout le reste et faites cet examen, sinon nous ne pouvons pas fixer la date de sortie". Et vous devez avoir le pouvoir de dire "désolé, nous n'avons pas pu sortir à temps car l'examen n'a pas été fait à temps".

Si ce n'est pas obligatoire, vous relâchez.

PS. Si Clint a un jour pour examiner des semaines ou des mois de travail, cela semble indiquer que l’examen n’était pas nécessaire. Mais si Clint trouve des problèmes dans ce court laps de temps, cela semble indiquer que les critiques précédentes n'étaient pas aussi bonnes qu'elles auraient dû l'être.

+1 - la responsabilité sans le pouvoir de vraiment faire les choses est un problème que j'ai vu plus souvent que je ne le voudrais.

Étant donné que "Clint" a apparemment eu moins d'une journée pour passer en revue les semaines-hommes ou peut-être les hommes-mois de travail, cette réponse semble tout à fait hors de propos.Rendre le réviseur "responsable" de la révision du code plus rapidement qu'il n'est en fait possible pour lui de le réviser est clairement déraisonnable.Et comme Clint a apparemment * réussi * à laisser des commentaires utiles dans les quelques heures qu'il a eues, il semble d'autant plus pervers de suggérer que tout serait réparé si seulement l'OP avait la capacité de forcer Clint à abandonner ses propres priorités et à revoirou blâmer Clint d'être trop lent.

@MarkAmery Je ne vois dans la question aucune indication que le temps de Clints était limité par OP.Si OP avait le pouvoir de demander "de combien de temps avez-vous besoin pour l'examen?"puis le pouvoir de dire «OK, donc vous devez commencer à la date X ou avant, car nous avons besoin de commentaires avant Y», alors ce serait OK.Si OP ne peut pas le faire, il ne devrait pas être tenu pour responsable de ne pas avoir répondu aux commentaires.Ce devrait être un problème de gestion, pas OP.Il ne s'agit pas de blâmer Clint, mais de ne pas obliger OP et son équipe à faire l'impossible.

+1 De plus, l'équipe d'OP est libre de répondre aux commentaires des non-bloquants avant la prochaine version.

@Mołot Peter Parker, l'oncle Ben était proche quand il a dit "avec une grande puissance vient une grande responsabilité".Ils ne se réunissent pas.C'est la même chose.Si je n'ai pas le pouvoir de contrôler comment quelque chose est fait, je ne suis pas responsable de la façon dont cela est mal fait.Trop de gens essaient de séparer le pouvoir et la responsabilité les uns des autres, mais cela ne peut tout simplement pas être fait et les tentatives pour le faire échouent.Chaque fois que quelqu'un essaie de me tenir responsable là où je n'ai aucun pouvoir, je dis "que pensez-vous que j'aurais pu faire différemment pour empêcher $ {BAD_THING}?";la réponse revient généralement à «rien».

@MontyHarder Redéfinir le terme «responsable» pour l'adapter à la discussion actuelle n'est guère bénéfique.S'ils vous posent des questions sur l'examen, et attendent votre * réponse *, et que vous répondez (principalement parce que vous liez en interne la réponse à votre image de soi professionnelle), alors c'est tout: vous êtes aussi * responsable * de la chose qu'elleobtient.Vous pouvez adapter une telle définition sans avoir le pouvoir ou le contrôle complet sur chaque aspect.

@Mołot Ma lecture de la séquence des événements était qu'il y avait moins de 24 heures entre l'ouverture du PR (et donc disponible pour Clint pour examen) et sa fusion malgré ses objections.Cette interprétation est maintenant confirmée par le PO.Il a été ouvert à un moment donné pendant une journée de travail, Clint a réussi à faire un examen, puis l'équipe a tout ignoré et a fusionné à 4h30 du matin le lendemain, l'OP accusant Clint de ne pas avoir examiné plus tôt le PR qui *n'existait pas encore *.Cela me semble assez fou.

@MarkAmery dans de telles circonstances, c'est un problème d'horaire.Et je pense toujours qu'OP devrait être en mesure de demander de combien de temps Clint a besoin et de planifier en conséquence ou, s'il n'a pas le pouvoir de succéder, OP ne devrait pas être blâmé pour le peu de temps dont Clint avait ou la revue des faits a été ignorée.La seule personne à blâmer est celle qui a conçu un calendrier aussi trop serré.

@Mołot Bien sûr, c'est peut-être hors des mains du PO.(Ou peut-être pas. Vous supposez, je pense, que le calendrier était hors du contrôle du PO, mais rien ne le corrobore dans la question; le PO gère une grande équipe et fixe ou au moins influence de manière plausible leur calendrier.)ne rachète pas une réponse qui blâme la situation spécifiquement et uniquement sur le manque de pouvoir de l'OP * sur Clint * lorsque l'OP reconnaît que Clint a immédiatement laissé des commentaires utiles le jour même où il a reçu quelque chose à revoir.Aucune puissance ne permettra à l'OP d'obliger Clint à revoir le code avant qu'il n'existe.

@MarkAmery D'après la question, Clint a été invité à donner des avis de code mais n'a pas accepté.Il n'y a aucune preuve qu'OP retient le code jusqu'à la fin du processus.Clint choisit apparemment de lui-même de réviser seulement à la fin du processus.La conclusion est que Clint ne pense pas que les contributions qu'il pourrait obtenir ne valent pas vraiment son temps.

@DavidThornley Oui, selon la question, Clint a * maintenant * été invité à des réunions de révision de code.Même si nous considérons son refus comme une preuve qu'il * choisit * de ne pas revoir jusqu'à la fin du processus (par opposition, par exemple, à des conflits avec des réunions plus importantes, ou à ce que son responsable lui demande de ne pas passer du temps à en faire une autretravail de l'équipe pour eux), cela n'a rien à voir avec l'incident antérieur décrit par l'OP.La conclusion ne suit pas du tout, puisque nous n'avons aucune information sur les raisons pour lesquelles Clint a refusé les réunions.

Joe McMahon

2018-11-21 05:03:14 UTC

view on stackexchange narkive permalink

On dirait qu'il y a un certain nombre de problèmes ici, mais ils sont tous réglables.

Problèmes de processus:

Quel est le but de cette pull request? Est-il destiné à être revu ou est-ce simplement un moyen de fusionner pour maîtriser à partir d'une branche contrôlée et testée? Si c'est le cas, vous devriez envisager une planification différente pour le processus d'examen. Dans ce dernier cas, il devrait être fusionné presque à l'instant où il a été créé.
Que faites-vous des commentaires de révision "tardifs"? Il semble que les commentaires de Clint ne sont pas passés par le processus d'examen complet, même s'ils étaient «en retard». (Qu'est-ce qui définit "en retard"?) Même si vous vous êtes engagé à fusionner et qu'aucun des commentaires ne fait obstacle, il devrait être possible de classer ses commentaires comme exploitables ou non et de répondre dans le PR le cas échéant. >

Problèmes personnels:

J'obtiens un peu une ambiance "nous ne sommes pas tous une équipe" ici. Cela peut provenir d'une frustration compréhensible de votre part, mais si vous respectez Clint - et je suppose que vous le faites - il est approprié d'essayer de résoudre ce problème. Il est peut-être en train de refuser vos demandes de réunion d'examen parce qu'il est occupé, ou il a peut-être l'impression que vous ne voulez vraiment pas de sa contribution de toute façon.
Remplissez pour montrer vos réelles intentions. Si vous pensez que certaines des choses qu'il a dites étaient de vrais problèmes, déposez des tickets et assurez-vous qu'il y est inclus.

Laisser le PR ouvert pour un "vrai avis" jusqu'à la veille du push signifie que soit vous devez prendre tous les avis au sérieux et ne pas fusionner et ne pas pousser s'il y a des commentaires non traités, ou vous devez modifier la planification du push afin qu'il y ait suffisamment de temps pour terminer la révision et pour planifier ou annuler le push (par exemple, les PR non fusionnés au moins 24 heures avant le push programmé ne sortent pas).

Ce serait peut-être une bonne idée de l'inviter à prendre un café et d'en discuter, en lui faisant savoir que oui, vous appréciez sa contribution, mais que le processus tel qu'il est actuellement signifiait que vous n'aviez pas obtenu son avis assez tôt pour agir cette fois, en insistant sur «cette fois». Faites-lui savoir que vous envisagez de modifier le processus et demandez-lui s'il a des suggestions.

Demandez-lui ce qui lui permettrait de contribuer plus facilement et assurez-vous qu'il comprend que vous appréciez qu'il soit dérangé. S'il essaie vraiment d'aider, alors aucune action sur ses commentaires ne le démotive également. On dirait qu'il n'y avait pas de bouchons, puisque vous avez poursuivi la fusion, mais il semble aussi que le travail, de son point de vue, était un effort inutile.

Remerciez-le d'avoir trouvé les problèmes qu'il a signalés et faites-lui savoir que vous avez l'intention d'aborder les choses importantes qu'il a soulevées. Vous devriez envisager de le faire à la fois en tête-à-tête avec lui et en public dans le cadre du PR fermé pour lui faire comprendre, ainsi qu'à l'équipe, que vous êtes heureux qu'il aide. Faire preuve de gratitude, en privé et en public, pour sa bienveillance et son bénévolat pour aider, même si aucun des commentaires d’évaluation n’était réalisable du point de vue de votre équipe, est juste la chose polie à faire, et cela aide à construire la solidarité entre les équipes.

Pour être plus précis sur la raison «occupée», Clint pourrait avoir l'impression d'être entraîné dans une équipe et une charge de travail qui n'est pas de sa responsabilité alors qu'il en a déjà assez (peut-être même trop!) À se soucier.Dans ce cas, l'inclure dans les rapports de bogue et faire d'autres efforts pour le faire se sentir plus impliqué / valorisé peut en fait nuire à la relation.

Les ingénieurs de support IME sont souvent surchargés.Il serait utile de connaître les raisons pour lesquelles Clint a refusé les invitations à la révision de code (il est peut-être simplement trop occupé).Faire savoir à Clint que ses contributions sont précieuses, même tardives, serait un bon moyen de le rapprocher de l'équipe de l'OP.Si Clint a des capacités, mais a l'impression de ne pas être assez proche pour s'impliquer plus tôt, il se peut qu'il se sente mal à l'aise de marcher sur les orteils des autres.

Ouais, c'est pourquoi tu sors prendre un café et en parle.Cette fois-ci, il semble avoir fait du bénévolat, donc s'enregistrer et voir si et comment il aimerait contribuer semble être le moyen le plus direct de régler les choses, et dans un cadre informel, il peut se défouler s'il se sent comme iln'était pas valorisé.

Joe Strazzere

2018-11-21 05:39:45 UTC

view on stackexchange narkive permalink

Comment puis-je dire à Clint s'il veut fournir des commentaires, cela doit être selon nos conditions?

À moins que Clint ne travaille pour vous, ou qu'il y ait une sorte de formel processus qui dicte quand les commentaires ne sont pas autorisés, vous ne pouvez pas forcer Clint à adhérer à "vos conditions".

Vous pouvez ignorer ses commentaires. Vous pouvez le remercier pour les commentaires «quelque peu utiles» pour en encourager davantage. Vous pouvez ajouter des éléments à votre backlog technique pour répondre à ces commentaires utiles. Vous pouvez continuer à l'inviter à des réunions de révision de code. Vous pouvez encourager votre équipe à ne pas se démotiver en raison de quelques commentaires tardifs.

Ertai87

2018-11-20 21:35:16 UTC

view on stackexchange narkive permalink

1) N'y a-t-il personne entre vous et 20 ingénieurs qui puisse vous aider à les encadrer? C'est pourquoi 20 personnes, c'est trop de personnes dans une équipe, car il n'y a aucun moyen qu'une seule personne puisse gérer et encadrer 20 personnes. Vous devez réduire la taille de votre équipe, ou du moins la taille de votre responsabilité, car vous êtes bien trop dispersé.

2) En ce qui concerne la publication du code, Clint fait-il partie de votre équipe? Si Clint fait partie de votre équipe, alors Clint devrait être impliqué dans la révision du code, surtout s'il est ingénieur senior. Vous devriez encourager vos mentorés à envoyer des révisions de code à Clint si possible (ne le surchargez pas, mais encouragez-les à impliquer Clint dans le processus). Si Clint ne fait pas partie de votre équipe, alors à moins que les problèmes que Clint trouve soient des bogues critiques, demandez-lui de les consigner sous forme de rapports de bogues; il ne fait pas partie de votre équipe, il n'est pas responsable de votre produit.

4) Quant à Clint, il manque des "réunions de révision de code": Premièrement, je ne sais pas ce qu'est une "réunion de révision de code". Les révisions de code sont des opérations asynchrones: le développeur soumet le code, le réviseur de code passe en revue pendant que le développeur fait autre chose, examine les finitions, adresse les commentaires du développeur, rince, répète. Je ne sais pas ce qu'est une "réunion de révision de code", cela semble idiot. Mais à côté de cela, si Clint fait partie de votre équipe, Clint devrait assister aux réunions d'équipe. Si Clint ne fait pas partie de votre équipe, Clint n'a pas besoin d'assister aux réunions d'équipe. C'est aussi simple que ça.

«Réunions de révision du code» - Nous avions l'habitude de faire ces réunions.Une fois par semaine, l'un des développeurs principaux passait par diverses demandes de fusion récentes et indiquait différents exemples de code particulièrement bon et mauvais, et en particulier ce qui était bon ou mauvais à ce sujet.Ensuite, tout le monde aurait une grande discussion à ce sujet.C'est un excellent moyen de distiller les connaissances de l'expérience en quelque chose que les juniors peuvent comprendre et absorber.

Les révisions de code peuvent être asynchrones.Il peut également s'agir de réunions de groupe où le code est parcouru.Il s'avère qu'avoir un groupe peut être plus productif car différentes personnes attrapent différentes choses.

ChrisW

2018-11-20 20:33:52 UTC

view on stackexchange narkive permalink

Quelqu'un doit définir le "processus", par exemple l'ordre dans lequel les choses se passent.

En tant que développeur, si je ne suis pas celui qui définit le processus, je ferai une révision du code (si c'est mon travail, comme prévu par mon responsable) lorsque le processus me dit de le faire et / ou quand quelqu'un me le demande.

Je ne comprends pas le passage de la question qui dit "a refusé toutes nos invitations à une réunion de révision de code ce mois-ci".

Je ne suis pas sûr de ce qu'est une "réunion de révision de code", d'ailleurs. Mon idée de la révision de code est:

Quelqu'un le code
Je l'examine (à mon rythme) et prends des notes
Je rencontre le codeur pour discuter de mon avis

Si je suis "senior", alors peut-être que je n'écoute pas les réunions d'examen des autres.

Pour gagner du temps (réduire mon effort) J'aime revoir le code après qu'il a été testé. Je pourrais toujours trouver des bogues (par exemple, si les tests ne sont pas parfaitement terminés), mais il est plus facile de revoir le code qui fonctionne que le code qui ne le fait pas. L'examen du code qui ne fonctionne pas s'appelle «développement et débogage» et prend plus de temps.

Pouvez-vous rendre les «tests d'intégration» plus faciles, plus automatisés peut-être? Pour que vous puissiez faire:

Développement
Tests d'intégration
Révision
Corriger les commentaires de révision
Test d'intégration à nouveau

Vous pouvez également fusionner avant la révision du code (si vous pensez que les tests d'intégration étaient suffisants sans examen), effectuez la révision du code sur la branche principale et utilisez les commentaires de révision du code comme entrée pour ce que vous pourriez souhaiter améliorer lors d'une itération ultérieure (un "sprint" ultérieur).

-1 pour "la révision du code doit être effectuée après le test".Certains tests peuvent être automatisés, mais beaucoup de tests sont manuels (cela est particulièrement vrai pour le code frontal, mais peut également l'être pour le code backend).Les tests manuels peuvent prendre beaucoup de temps;les revues de code sont généralement courtes.

Par «après les tests», je veux dire, au minimum, après avoir réexécuté [tests de fumée automatisés] (https://en.wikipedia.org/wiki/Smoke_testing_ (logiciel)) plus tout ce que vous faites pour tester que la nouvelle fonctionnalité fonctionne comme spécifié.Différents systèmes ont des niveaux d'automatisation différents dans leurs tests (et j'ai dit que si leurs tests étaient plus automatisés, ils pourraient se permettre de le faire plus souvent).

Les tests en cours de développement sont différents des tests en préparation pour la publication et, selon les tests, ils peuvent prendre un temps considérable, vous voulez donc limiter la fréquence à laquelle vous les exécutez.

@JoeW Oui et "la nuit avant une grosse sortie" sonne plutôt [trop] tard.Je suppose que "les commentaires allaient de quelque peu utiles à quelque peu inutiles" signifie qu'il n'a pas trouvé d'obstacles, et donc l'OP a eu raison de sortir à temps quand ils l'ont fait.Et ce n'est pas mal du tout - le fait qu'une révision de code ne trouve pas d'obstacles n'est pas nécessairement une mauvaise chose.Quel est * est * son travail, sinon «enseigner à mes ingénieurs»?S'agit-il simplement de regarder (c'est-à-dire de fumer) les changements dans la branche d'intégration avant la publication?

@ChrisW Je suis tout à fait d'accord pour dire que le code non testé ne doit pas être revu (j'irais encore plus loin et dirais qu'il ne devrait pas du tout être validé!)Cependant, comme Joe l'a dit, les tests qu'un développeur devrait être censé exécuter sur son code sont différents des tests qu'un QA exécuterait.Le premier, bien sûr.Ce dernier, pas tellement.

Je pense que vous avez manqué un problème critique avec les tests avant de revoir le code lui-même.Si les tests d'intégration / de fumée auront un temps d'exécution significatif, est-il judicieux d'exécuter les tests et de faire un examen par les pairs qui pourrait trouver des problèmes nécessitant que les tests soient à nouveau exécutés?N'aurait-il pas plus de sens de faire l'examen avant la main afin que tout problème potentiel puisse être détecté avant le test, afin de minimiser la fréquence à laquelle vous devez exécuter ces tests?

@JoeW Je suppose que vous avez raison, c'est-à-dire que l'OP impliquait que ses "tests d'intégration" étaient coûteux ou longs - alors peut-être que le "processus" devrait spécifier que l'inspection doit être effectuée avant les tests d'intégration?Si c'est avant les tests d'intégration, est-ce avant ou après l'intégration (je ne sais pas)?J'essayais peut-être de trouver des excuses pour Clive, c'est-à-dire deviner quels pourraient être les motifs de Clive.S'il veut revoir si tard, alors peut-être que son inspection devrait être considérée comme faisant partie du test d'intégration, c'est-à-dire simplement "go / no-go", et tous ses commentaires de style semi-utiles rejetés ...

... ou triées, par ex.l'OP pourrait éventuellement les ajouter aux directives de révision de code de son équipe, ou les utiliser pour la formation à la révision de code.

"La révision du code qui ne fonctionne pas est appelée" développement et débogage "et prend plus de temps."Je pense que c'est plus un compromis, et cela pourrait dépendre de la capacité / expérience des développeurs.L'examen avant les tests d'assurance qualité peut permettre d'économiser globalement si vous trouvez des problèmes majeurs qui impliquent une refonte majeure du travail;il ne sert à rien de tester du code qui doit être largement jeté (par exemple, vous découvrez que du code récupérant quelques milliers de résultats de rapport saisit les données associées un enregistrement à la fois).J'essaie de ne pas rechercher de vrais bugs, mais plutôt de me concentrer sur des choses comme l'approche globale et les normes.

Aleksandr Dubinsky

2018-11-22 06:22:43 UTC

view on stackexchange narkive permalink

Je suis sûr que ce type vous dit simplement que vous faites du mauvais travail. C'est ce que signifie «ce n'est pas mon travail d'enseigner à vos ingénieurs». Il n'est pas intéressé à faire votre travail pour vous, il veut que vous le fassiez mieux. C'est pourquoi il a examiné le code destiné à la production (code sur lequel vous vous êtes déconnecté), et non le code en cours de développement. Quant à savoir quoi faire à ce sujet, eh bien, c'est un sujet pour une autre question.

Je pense honnêtement que cela touche le clou sur la tête.La plupart des personnes qui ne font pas partie de votre équipe ne veulent pas de travail supplémentaire de votre équipe.Ils veulent juste que votre équipe fasse du bon travail pour ne plus avoir à y penser.

Je conviens que c'est un scénario probable.Puisque "Clint" a travaillé plus longtemps sur la base de code, il la connaît probablement mieux aussi.

Joshua

2018-11-21 02:43:07 UTC

view on stackexchange narkive permalink

Méfiez-vous.

✓ Développeur senior
✓ Détecte des problèmes dans le code que personne d'autre n'a fait
✓ L'équipe découragée en raison du retard de signalement
✓ Signale quelque chose à propos de "pas son travail" mais revu quand même
✗ se prépare pour une répétition du même scénario

Cela ne semble pas très bon pour l'équipe, ni pour le développeur principal non plus.

Mais je vous dis de prendre garde. Assurez-vous que vous comprenez tous les commentaires sur cet avis d'extraction. Et je veux dire vraiment comprendre. "C'est tout simplement faux." ne compte que si vous avez vraiment dépensé l'effort de vérifier.

Il n'y a probablement rien de majeur qui n'attend que vous soyez destructeur. Mais il pourrait y avoir un bug de perte de données extrêmement subtil qu'il a trouvé qui ne peut pas être révélé lors des tests. Vous ne saurez pas la différence tant que vous n’aurez pas compris tous les commentaires.

Addendum: pour restaurer le moral de l’équipe, assurez-vous de disposer de suffisamment de temps pour résoudre tous les problèmes que le développeur principal a trouvés dans cette demande d’extraction que le le reste de l'équipe est d'accord que cela devrait être corrigé.

Joe W

2018-11-21 01:42:39 UTC

view on stackexchange narkive permalink

Le problème ici est que votre processus de workflow est défectueux. Les examens par les pairs doivent avoir lieu avant les tests d'intégration car le simple fait d'attendre la fin des tests peut ralentir le processus. Si l'examen par les pairs trouve des problèmes qui nécessitent des modifications et que les tests d'intégration sont déjà effectués, il faudra plus de temps pour revenir en arrière et refaire tous les tests une fois les modifications terminées.

Idéalement, le développeur devrait écrire et effectuer des tests au fur et à mesure qu'ils développent le code et que tout test final doit être effectué après que le code a été examiné par les pairs pour s'assurer qu'il fonctionne comme prévu. Une chose à retenir est qu'un examen par les pairs peut repérer des bogues dans le code avant de pouvoir arrêter un système pendant les tests d'intégration.

"Le problème ici est que votre processus de flux de travail est défectueux."Je ne comprends pas cette perspective.Pour moi, il semble que l'équipe essaie de faire des évaluations par les pairs tôt, et le problème est que Clint les ignore jusqu'à la dernière minute.Le processus de workflow semble correct, mais Clint semble refuser de participer efficacement.

@DarthFennec Pourquoi voudriez-vous faire des tests d'intégration avant de faire un examen par les pairs du code?Cela me semble défectueux car vous devrez répéter l'un des tests qui ont été effectués si un problème est détecté avec le code dans la revue.

Je ne suis pas en désaccord avec cela.Je dis qu'OP ne semble pas non plus en désaccord."le lendemain, j'ai demandé s'il aurait pu revoir ce code plus tôt au lieu d'attendre après les tests d'intégration."Il semble qu'OP et son équipe essaient de faire l'examen par les pairs avant les tests d'intégration, et le problème déclaré est que le réviseur (Clint) agit contre ce processus de flux de travail.

@DarthFennec Une partie du problème ici est le manque de détails de la question originale qui nous permet de lire des récits sous-jacents très différents dans l'histoire.Votre lecture, je suppose, est que l'équipe voulait que Clint examine, mais il s'est dérobé à ses fonctions jusqu'à la dernière minute.Ma lecture était que Clint n'était pas impliqué dans le projet et n'avait aucune autorité sur celui-ci, puis s'est retrouvé pris dans une embuscade le jour de la date limite avec un PR géant d'hommes-mois de code de merde qu'il n'avait eu aucune chance de revoir, puisl'OP s'est ennuyé de la rétroaction "tardive" et l'a fusionnée malgré les objections de Clint.

@MarkAmery "a ensuite été pris en embuscade le jour de la date limite" cela ne semble-t-il pas contredire le comportement d'OP?Pourquoi OP aurait-il dit "J'ai demandé s'il aurait pu revoir ce code plus tôt au lieu d'attendre après les tests d'intégration" si Clint était pris en embuscade avec le code après les tests d'intégration?Pourquoi la réponse de Clint serait-elle "ce n'est pas mon travail" au lieu de "je n'en savais rien avant les tests d'intégration"?Je ne dis pas que vous vous trompez, je ne comprends tout simplement pas comment vous êtes arrivé à votre conclusion.Je conviens que cela aurait été mieux si OP avait été plus clair à ce sujet, cependant.

@DarthFennec Je suis d'accord que l'attitude du PO semble irrationnelle compte tenu de mon interprétation, mais elle est confirmée par la modification du PO.La séquence des événements est la suivante: 1. un groupe de développement dans lequel Clint n'a aucun rôle est effectué par l'équipe de l'OP, 2. le communiqué de presse est ouvert, 3. Clint est amené à un contrôle de cohérence et le fait le même jour, laissantquelques commentaires négatifs, 4. la libération a lieu à 4h30 du matin de toute façon, 5. OP se plaint à Clint de la rétroaction "tardive".Le commentaire "ce n'est pas mon travail" semble raisonnable de la part de quelqu'un qui est allé au-delà de l'appel du devoir pour laisser de nombreux commentaires aux membres de ...

@DarthFennec ... une autre équipe de développement, puis a été réprimandée pour cela et a dit qu'elle aurait dû en faire plus, et plus tôt, sur un projet qui n'était pas le leur au départ.La réponse du PO à cela semble juste comme un transfert de blâme insignifiant sans beaucoup de justification derrière cela - ce qui est, au moins, quelque chose d'humain et de compréhensible.

@MarkAmery De l'édition: "nous ouvrons un PR pour fusionner avec Master ** après que ** les ingénieurs approuvent les changements" J'interprétais cela comme signifiant que Clint était l'un de ces ingénieurs, et il a attendu d'approuver les changements jusqu'après le PR pourMaster a été créé, au lieu de le faire avant ou en parallèle avec les tests d'intégration d'AQ comme il était censé le faire.Mais cela pourrait vraiment aller dans les deux sens.Le fait qu'on ait demandé à OP quel était le rôle de Clint à plusieurs reprises et qu'il n'ait pas répondu directement à cela dans l'édition est suspect;s'il évite la question exprès, cela donne du crédit à l'interprétation de rejet de blâme.

@MarkAmery Rien n'indique que la contribution de Clint était requise.(En effet, le déploiement a été fait avant que ses commentaires ne puissent être pris en compte.) OP invite Clint à réviser le code et craint que les commentaires de dernière minute qui ne peuvent pas être utiles pour cette version ne se reproduisent.OP veut évidemment que Clint aide à éduquer son équipe, et évidemment Clint ne veut pas le faire.Toute accusation de transfert de blâme devrait vraiment montrer que le blâme existe en premier lieu, et je ne vois pas cela.

Karl Bielefeldt

2018-11-21 20:49:41 UTC

view on stackexchange narkive permalink

Vous avez deux types d'avis différents (trois si vous comptez votre réunion en personne). Ce n'est pas intrinsèquement mauvais, mais cela signifie que vous devez clarifier vos attentes pour chaque circonstance. Je créerais un modèle de demande d'extraction pour votre examen final qui ressemblerait à ceci. Cela rend très clair non seulement ce qui est attendu de cet examen, mais aussi comment obtenir plus de succès pour vos commentaires à l'avenir.

Ceci est une revue de pré-production. Son objectif est de trouver des problèmes majeurs tels que:

Erreurs de fusion
Ajout accidentel d'une fonctionnalité incomplète
Bogues Showstopper

Sauf ces types de problèmes, cette pull request sera fusionnée à <date et time>.

Les revues de conception individuelles, où nous discutons de la lisibilité, de la maintenabilité et de l'architecture globale, sont déjà terminées. Si vous trouvez ce genre de problèmes ici, veuillez ouvrir un problème ou apporter les modifications dans votre propre pull request au développement au lieu d'encombrer cette pull request. Les revues de conception ont été effectuées dans les pull requests suivantes:

<Link to PR 1>
<Link to PR 2>