Comment gérer un désaccord dans une révision de code concernant un cas marginal improbable?

Je travaille dans une startup de la robotique dans une équipe de couverture de chemin et après avoir soumis une demande d'extraction, mon code est examiné.

Mon coéquipier, qui fait partie de l'équipe depuis plus d'un an, a fait quelques remarques dans mon code qui suggèrent que je fasse beaucoup plus de travail que je ne le crois nécessaire. Non, je ne suis pas un développeur paresseux. J'aime le code élégant qui contient de bons commentaires, des noms de variable, une indentation et gère correctement les cas. Cependant, il a en tête un autre type d'organisation avec lequel je ne suis pas d'accord.

Je vais donner un exemple:

J'avais passé une journée à écrire des scénarios de test pour modifier un algorithme de recherche de transition que j'avais créé. Il m'avait suggéré de traiter un cas obscur qui aurait très peu de chances de se produire - en fait, je ne suis pas sûr que cela soit possible. Le code que j'ai créé fonctionne déjà dans tous nos scénarios de test d'origine et dans certains nouveaux que j'ai trouvés. Le code que j'ai créé passe déjà plus de 300 simulations exécutées tous les soirs. Cependant, gérer ce cas obscur me prendrait 13 heures qui pourraient être mieux dépensées à essayer d’améliorer les performances du robot. Pour être clair, l’algorithme précédent que nous utilisions jusqu’à présent ne traitait pas non plus ce cas obscur, et pas une seule fois, dans les 40 000 rapports générés. Nous sommes une start-up et devons développer le produit.

Je n'ai jamais eu de révision de code auparavant et je ne suis pas sûr de trop argumenter; devrais-je me taire et faire ce qu'il dit? J'ai décidé de garder la tête basse et de faire le changement même si je suis tout à fait en désaccord sur le fait que c'était une bonne utilisation du temps.

Je respecte mon collègue et je le reconnais comme un programmeur intelligent. Je ne suis tout simplement pas d'accord avec lui sur un point et je ne sais pas comment gérer un désaccord dans une révision du code.

Je pense que la réponse que j'ai choisie répond à ce critère, qui consiste à expliquer comment un développeur débutant peut gérer un désaccord lors d'une révision de code.

un cas obscur qui est extrêmement peu probable - en fait, je ne suis pas sûr que cela soit possible

Ne pas avoir de comportements non testés dans le code peut être très important. Si un morceau de code est exécuté, par exemple 50 fois par seconde, une chance sur un million se produira environ toutes les 5,5 heures d’exécution. (Dans votre cas, les chances semblent plus faibles.)

Vous pouvez parler des priorités avec votre responsable (ou toute autre personne responsable de l’unité dans laquelle vous travaillez). Vous comprendrez mieux si, par exemple, travailler sur les performances du code ou si le code est à l'épreuve des balles, constitue la priorité absolue, et à quel point ce cas de figure peut être improbable. Votre critique peut également avoir une idée faussée des priorités. Après avoir parlé avec la personne en charge, vous aurez plus de facilité à être en désaccord avec vos suggestions de relecteur et vous aurez quelque chose à consulter.

C'est toujours une bonne idée d'avoir plus d'un critique. Si votre code n’est revu que par un collègue, demandez à une autre personne connaissant ce code, ou à la base de code en général, d’y jeter un coup d’œil. Une deuxième opinion, encore une fois, vous aidera à (plus) facilement être en désaccord avec les suggestions du critique.

Avoir un certain nombre de commentaires récurrents au cours de plusieurs revues de code indique généralement qu'une chose plus importante n'est pas clairement communiquée et que les mêmes problèmes se répètent encore et encore. Essayez de trouver cette chose plus importante et discutez-en directement avec le critique. Demandez assez pourquoi des questions. Cela m'a beaucoup aidé lorsque j'ai commencé la pratique de la révision de code.

Je peux vous donner un exemple d'un cas de coin qui pourrait ne jamais se produire et qui a provoqué un désastre.

Lors du développement de l’ Ariane 4, les valeurs des accéléromètres latéraux ont été mises à l’échelle pour s’inscrire dans un entier signé de 16 bits et, dans la mesure où la sortie maximale possible des accéléromètres, une fois mise à l’échelle, ne peut dépasser 32767 et le minimum ne peut jamais être inférieur à - 32768, il n’y avait «pas besoin de frais généraux de vérification de la plage». En général, toutes les entrées sont supposées être vérifiées dans une plage avant toute conversion, mais dans ce cas, cela équivaudrait à saisir un cas de coin impossible.

Plusieurs années plus tard, Ariane 5 était en cours de développement et le code de dimensionnement des accéléromètres latéraux était réutilisé avec des tests minimaux car il avait été «testé». Malheureusement, la plus grande fusée pouvait s'attendre à des accélérations latérales plus importantes. Les accéléromètres ont donc été mis à niveau et peuvent générer des valeurs de flottement plus importantes sur 64 bits .

Ces valeurs plus grandes "encapsulées" dans le code de conversion, ne souviennent pas de vérification de la plage, et les résultats du premier lancement en 1996 n'étaient pas bons. Cela a coûté des millions de dollars à la société et provoqué une interruption importante du programme.

Le point que je suis en train de faire est que vous ne devriez pas ignorer les cas de test que jamais se produire, très improbable, etc .: les normes qu'ils codant pour a appelé à la vérification de gamme de toutes les valeurs d'entrée externes. Si cela avait été testé et traité, la catastrophe aurait peut-être été évitée.

Notez que dans Ariane 4, ce n’était pas un bug (car tout fonctionnait bien pour toutes les valeurs possibles) , c’était un manquement aux normes. Lorsque le code était réutilisé dans un scénario différent, il échouait de manière catastrophique. Si la plage de valeurs avait été découpée, il l'aurait probablement bien échoué, et l'existence d'un scénario de test pour cela aurait peut- être déclenché un examen des valeurs. Il convient également de noter que, si les enquêteurs ont critiqué les enquêteurs après l'explosion, les responsables de la gestion, de l'assurance de la qualité et des dirigeants ont été à l'origine de la plupart des accusations.

Clarification

Même si tous les logiciels ne sont pas critiques pour la sécurité, ni aussi spectaculaires quand ils échouent, mon intention était de souligner que des tests "impossibles" peuvent toujours avoir de la valeur. C'est le cas le plus dramatique que je connaisse, mais la robotique peut aussi produire des résultats désastreux.

Personnellement, je dirais qu’une fois l’opération mise en évidence par l’équipe de test, un test doit être mis en place pour le vérifier. Le responsable de l'équipe de mise en œuvre ou le chef de projet peut décider de ne pas tenter de remédier aux défaillances constatées, mais doit être informé de l'existence d'éventuelles lacunes. Alternativement, si les tests sont trop complexes ou trop coûteux à mettre en œuvre, un problème peut être soulevé dans le système de suivi utilisé et / ou dans le registre des risques pour indiquer clairement qu'il s'agit d'un cas non testé - qu'il peut être nécessaire de le résoudre. avant un changement d'utilisation ou empêcher une utilisation inappropriée.

Comme il n'a pas été traité auparavant, vos efforts sont hors de portée. Vous ou votre collègue pouvez demander à votre responsable s’il vaut la peine de s’efforcer de couvrir ce cas.

Avec des algorithmes complexes, il est très difficile de prouver que vous avez pensé à tous les cas de test pouvant survenir dans le monde réel. Lorsque vous laissez intentionnellement un cas de test cassé parce qu'il ne sera pas abordé dans le monde réel, vous risquez de laisser d' autres cas de test cassés auxquels vous n'avez même pas encore pensé.

L’autre effet qui se produit souvent est que lorsque vous gérez des cas de test supplémentaires, votre algorithme devient nécessairement plus général. De ce fait, vous trouvez le moyen de le simplifier et de le rendre plus robuste pour tous vos cas de test. Cela vous fait gagner du temps en maintenance et en dépannage.

En outre, il y a énormément de cas de bugs "cela ne devrait jamais arriver" dans la nature. En effet, votre algorithme ne changera peut-être pas, mais ses entrées pourraient changer des années plus tard, lorsque personne ne se souviendra de ce cas d'utilisation non géré. C'est pourquoi les développeurs expérimentés gèrent ce genre de choses quand ils sont frais dans leur esprit. Il revient vous mordre plus tard si vous ne le faites pas.

Ce n'est pas une question technique mais une décision de stratégie commerciale. Vous remarquez que la solution suggérée concerne un cas très obscur qui n'arrivera presque jamais. Ici, cela fait une grande différence si vous programmez un jouet ou si vous programmez un équipement médical ou un drone armé. Les conséquences d'un dysfonctionnement rare seront très différentes.

Lorsque vous effectuez des révisions de code, vous devez appliquer une compréhension de base des priorités commerciales lorsque vous décidez combien vous souhaitez investir dans le traitement des cas rares. Si vous n'êtes pas d'accord avec votre collègue dans votre interprétation des priorités de l'entreprise, vous souhaiterez peut-être discuter avec quelqu'un de votre entreprise.

Les revues de code ne concernent pas uniquement l'exactitude du code. En réalité, cela est assez loin de la liste des avantages, derrière le partage des connaissances et comme un processus lent mais régulier vers la création d'un consensus style / design d'équipe.

Comme vous le constatez, ce qui compte comme "correct" est souvent discutable, et chacun a son propre angle sur ce que c'est. D'après mon expérience, un temps et une attention limités peuvent également rendre les révisions de code très incohérentes - le même problème peut être détecté ou non en fonction de différents développeurs et à des moments différents par le même développeur. Les réviseurs dans l’état d’esprit de "ce qui améliorerait ce code?" suggérera souvent des changements qu’ils n’ajouteraient pas naturellement à leurs propres efforts.

En tant que codeur expérimenté (plus de 15 ans en tant que développeur), je suis souvent passé en revue par des codeurs moins expérimentés que moi. Parfois, ils me demandent des changements avec lesquels je suis légèrement en désaccord ou que je considère sans importance. Cependant, je fais toujours ces changements. Je ne me bats contre des modifications que lorsque je sens que le résultat final entraînera une faiblesse du produit, lorsque le coût en temps est très élevé ou qu'un point de vue "correct" peut être rendu objectif (par exemple, la modification demandée est gagnée). t fonctionne dans la langue que nous utilisons, ou un point de repère montre qu’une amélioration de performance revendiquée n’en est pas une).

Je suggère donc de choisir vos batailles avec soin. Deux jours de codage d’un scénario de test que vous estimez inutile, ne méritent probablement pas le temps et les efforts nécessaires pour vous battre. Si vous utilisez un outil de révision, tel que les demandes d'extraction GitHub, alors commentez peut-être les coûts / avantages que vous percevez, afin de noter votre objection, mais acceptez de continuer à travailler. Cela compte comme une légère poussée, de sorte que l'examinateur sait qu'il dépasse une limite et, plus important encore, explique votre raisonnement pour que des cas comme celui-ci puissent être escaladés s'ils se retrouvent dans une impasse. Vous souhaitez éviter une escalade des désaccords écrits - vous ne voulez pas avoir d'argument de style forum Internet au-dessus des différences de travail - il peut donc être utile de parler de la question en premier et d'enregistrer un résumé fidèle du résultat de la discussion,discussion amicale décidera pour vous deux).

Après l’événement, c’est un bon sujet pour la révision du sprint ou les réunions de planification de l’équipe de développement, etc. Présentez-le aussi neutre que possible. Par exemple, dans la révision du code, le développeur A a identifié ce scénario de test supplémentaire. Deux jours supplémentaires ont été nécessaires pour écrire. L’équipe pense-t-elle que la couverture supplémentaire était justifiée à ce prix? " - cette approche fonctionne beaucoup mieux si vous faites réellement le travail, car elle vous montre de manière positive; vous avez effectué le travail et souhaitez simplement interroger l'équipe sur l'aversion pour le risque et le développement de fonctionnalités.

Je vous conseillerais au moins d'affirmer contre le cas obscur. De cette façon, non seulement les futurs développeurs verront que vous avez activement décidé de vous opposer à l'affaire, mais avec une bonne gestion des défaillances, qui devrait déjà être en place, cela pourrait également vous surprendre.

Et puis, faites un cas de test qui affirme cet échec. De cette façon, le comportement est mieux documenté et apparaîtra dans les tests unitaires.

Cette réponse suppose évidemment que votre jugement du cas même "extrêmement improbable si possible" est correct et que nous ne pouvons pas en juger. Mais si c'est le cas, et votre collègue est d'accord, une affirmation explicite contre l'événement devrait être une solution satisfaisante pour vous deux.

Puisque vous semblez être nouveau, il n’ya qu’une chose que vous puissiez faire: vérifiez auprès du chef d’équipe (ou du chef de projet). 13 heures est une décision commerciale; pour certaines entreprises / équipes, beaucoup; pour certains, rien. Ce n'est pas votre décision, pas encore.

Si le chef de file dit "couvrir cette affaire", bien; s'il dit "non, vas-y", bien - sa décision, sa responsabilité.

En ce qui concerne les critiques de code en général, détendez-vous à ce sujet. Avoir une tâche qui vous est retournée une ou deux fois est parfaitement normal.

Une chose que je ne pense pas avoir abordée de la sorte, même si cela a été évoqué dans la réponse de SteveBarnes:

Quelles sont les répercussions d'un échec?

Dans certains champs, un échec est une erreur sur une page Web. Un PC bleu écrans et redémarre.

Dans d’autres domaines, c’est la vie ou la mort - une voiture autonome se bloque. Le stimulateur médical cesse de fonctionner. Ou dans la réponse de Steve: les choses explosent, entraînant la perte de millions de dollars.

Il y a un monde de différence dans ces extrêmes.

Que ce soit ou non 13 heures pour couvrir un "échec" vaut finalement la peine. Ce devrait être la direction et les propriétaires. Ils devraient avoir une idée de la situation dans son ensemble.

Vous devriez pouvoir deviner ce qui en vaudra la peine. Votre robot va-t-il simplement ralentir ou s'arrêter? Performance dégradée? Ou une défaillance du robot causera-t-elle des dommages monétaires? La perte de la vie?

La réponse à CETTE question devrait conduire à la réponse "est-ce que cela vaut 13 heures de temps par les entreprises". Remarque: j'ai dit l'heure des compagnies. Ils paient les factures et décident finalement de ce qui en vaut la peine. Votre direction devrait avoir le dernier mot de toute façon.

Peut-être parler à la personne responsable de la hiérarchisation des tâches? Au démarrage, il pourrait s'agir du CTO ou du propriétaire du produit. Il pourrait aider à déterminer si ce travail supplémentaire est nécessaire et pourquoi. Vous pouvez aussi apporter vos soucis lors des affrontements quotidiens (si vous en avez).

S'il n'y a pas de responsabilité claire (par exemple, le produit propriétaire) pour le travail de planification, essayez de parler aux gens autour de vous. Plus tard, le fait que tout le monde pousse le produit dans une direction opposée pourrait devenir un problème.

La meilleure façon de gérer les désaccords est la même, que vous soyez développeur junior, développeur senior ou même PDG.

Agir comme Columbo .

Si vous n’avez jamais regardé Columbo, c’était une série fantastique. Columbo était ce personnage très modeste - la plupart des gens pensaient qu'il était un peu fou et qu'il ne valait pas la peine de s'inquiéter. Mais en paraissant humble, et en demandant simplement aux gens d'expliquer, il a réussi à trouver son homme.

Je pense que cela est également lié à la méthode socratique .

En général, vous voulez vous poser des questions à vous-même et aux autres pour vous assurer que vous faites les bons choix. Pas à partir d'une position de "J'ai raison, vous avez tort", mais d'une position de découverte honnête. Ou du moins du mieux que vous pouvez.

Dans votre cas, vous avez deux idées ici, mais elles ont fondamentalement le même objectif: améliorer le code.

Vous avez l’impression que le meilleur moyen de le faire consiste à lésiner sur la couverture de code pour un cas potentiellement improbable (impossible?) En faveur du développement d’autres fonctionnalités.

Votre collègue a l’impression qu’il est plus précieux de faire plus attention aux coudes.

Que voient-ils que vous ne voyez pas? Que voyez-vous qu'ils ne voient pas? En tant que développeur junior, vous êtes en fait dans une excellente position car vous devez naturellement poser des questions. Dans une autre réponse, quelqu'un mentionne à quel point un cas de coin est étonnamment probable. Vous pouvez donc commencer par: «Aidez-moi à comprendre - j’avais l’impression que X, Y et Z - que me manque-t-il? Pourquoi le widget va-t-il se frotter? J'avais l’impression qu’il fintuerait dans des circonstances difficiles. le bâton magique a-t-il embigré les brosses ANZA? "

Lorsque vous remettez en question vos hypothèses et vos conclusions, vous allez creuser, découvrir les préjugés et, éventuellement, déterminer quelle est la marche à suivre correcte.

Commencez par supposer que tous les membres de votre équipe sont parfaitement rationnels et qu'ils ont à coeur les meilleurs intérêts de l'équipe et du produit, tout comme vous. S'ils font quelque chose qui n'a pas de sens, alors vous devez déterminer ce que vous ne savez pas qu'ils font ou ce que vous savez qu'ils ne font pas.

13 heures, ce n'est pas si grave, je le ferais. Rappelez-vous que vous êtes payé pour cela. Il suffit de le qualifier de "sécurité de l'emploi". Aussi, il est préférable de garder un bon karma au sein de l'équipe. Maintenant, si cela vous prenait une semaine ou plus, vous pourriez impliquer votre responsable et lui demander si ce serait la meilleure utilisation de votre temps, surtout si vous n'êtes pas d'accord avec cela.

Cependant, vous semblez avoir besoin de poids dans votre groupe. Voici comment vous obtenez un effet de levier: demandez pardon, ne demandez pas la permission. Ajoutez des éléments au programme à votre convenance (dans le cadre du cours, c’est-à-dire, assurez-vous qu’il résout complètement le problème recherché par le responsable ..), puis informez le responsable ou vos collègues par la suite. Ne leur demandez pas: "Est-ce que ça va si j'ajoute la fonctionnalité X". Ajoutez simplement les fonctionnalités que vous souhaitez personnellement dans le programme. S'ils s'énervent face à une nouvelle fonctionnalité ou s'ils ne sont pas d'accord avec cela, n'hésitez pas à la supprimer. S'ils l'aiment, gardez-le.

En outre, chaque fois qu'ils vous demandent de faire quelque chose, faites un effort supplémentaire et ajoutez beaucoup de choses qu'ils ont oublié de mentionner ou des choses qui fonctionneraient mieux que ce qu'ils ont dit. Mais ne leur demandez pas si c'est "ok" de faire un effort supplémentaire. Faites-le et dites-leur simplement que c'est fini. Qu'est-ce que vous faites est leur formation ..

Ce qui se passera sera que votre responsable vous considérera comme un "partant" et commencera à vous faire confiance, et vos collègues commenceront à vous voir comme le chef de file avant que vous ne deveniez le propriétaire du programme. Et ensuite, lorsque des choses comme ce que vous mentionnez se produisent dans le futur, vous aurez plus à dire parce que vous êtes essentiellement la star de l'équipe et que vos coéquipiers reculeront si vous n'êtes pas d'accord avec eux.

La révision du code a plusieurs objectifs. Vous savez évidemment que l’un de ces codes est le suivant: " Ce code est-il adapté à votre objectif? " En d’autres termes, est-il fonctionnellement correct; Est-il testé de manière adéquate? les parties non évidentes sont-elles commentées de manière appropriée? est-il conforme aux conventions du projet?

Une autre partie de la révision du code est le partage des connaissances sur le système. C'est à la fois une opportunité pour l'auteur et pour le relecteur de se familiariser avec le code modifié et son interaction avec le reste du système.

Un troisième aspect est qu’il peut permettre de passer en revue les problèmes qui existaient avant que des modifications ne soient apportées. Assez souvent, lorsque je passe en revue les modifications de quelqu'un d'autre, je repère quelque chose que j'ai manqué lors d'une précédente itération (très souvent, quelque chose de propre). Une déclaration telle que "Voici une occasion de rendre cela plus solide qu’elle ne l’était", n’est pas une critique, ne la prenez pas comme une seule!

Mon équipe actuelle considère la révision de code non seulement comme une passerelle ou un obstacle à la réussite du code, mais plutôt comme une opportunité pour une discussion quelque peu structurée d'un domaine de fonctionnalité particulier. C'est l'une des occasions les plus productives pour le partage d'informations. (Et c’est une bonne raison de partager la critique autour de l’équipe, plutôt que de toujours la même critique).

Si vous percevez la révision de code comme une activité conflictuelle, c'est une prophétie auto-réalisatrice. Si au contraire, vous les considérez comme la partie la plus collaborative de votre travail, ils stimuleront des améliorations continues de votre produit et de la manière dont vous travaillez ensemble. Il est utile, si un examen peut définir clairement les priorités relatives de ses suggestions - par exemple, il y a beaucoup de différence entre "j'aimerais un commentaire utile" et "Cela se casse si xc'est négatif", par exemple.

Après avoir formulé beaucoup de déclarations générales, comment cela s’applique-t-il à votre situation particulière? J'espère qu'il est maintenant évident que mon conseil est de répondre à l'examen en posant des questions ouvertes et de négocier quelle approche a le plus de valeur. Dans votre exemple, dans le cas où un test supplémentaire est suggéré, quelque chose du type "Oui, nous pouvons le tester; j’estime que la mise en oeuvre prendra <temps> . Pensez-vous que le bénéfice en vaut la peine? Et y at-il autre chose que nous peut faire pour garantir que le test n'est pas nécessaire? "

En lisant votre question, une chose me frappe: s'il faut deux jours d'effort pour écrire un nouveau cas de test, votre nouveau test est un scénario très différent de vos tests existants (dans ce cas, il a probablement beaucoup valeur) ou si vous avez identifié le besoin d’une meilleure réutilisation du code dans la suite de tests.

Enfin, en tant que commentaire général sur la valeur des révisions de code (et en tant que résumé concis de ce que j'ai dit ci-dessus), j'aime bien cette déclaration dans Maintainers Don't Scale de Daniel Vetter :

Au moins pour moi, l'examen ne consiste pas uniquement à garantir une bonne qualité de code, mais également à diffuser des connaissances et à améliorer la compréhension. Au début, il y a peut-être une personne, l'auteur (et ce n'est pas une donnée), qui comprend le code. Après un bon examen, il devrait y avoir au moins deux personnes qui le comprennent parfaitement, y compris les cas critiques.

Le code peut TOUJOURS être meilleur.

Si vous êtes dans une révision de code et que vous ne voyez rien qui pourrait être meilleur ou un test unitaire susceptible de détecter un bogue, ce n'est pas le code qui est parfait, mais le réviseur qui ne fait pas son travail. Que vous choisissiez de mentionner l'amélioration ou non, c'est un choix personnel. Mais presque chaque fois que votre équipe procède à une révision du code, il devrait y avoir des choses que quelqu'un remarque qui pourraient être meilleures ou tout le monde a probablement perdu leur temps.

Cela dit, que vous agissiez en fonction des commentaires ou non, cela dépend de votre équipe. Si vos modifications résolvent le problème ou ajoutent suffisamment de valeur sans modification pour que votre équipe les accepte, fusionnez-les et enregistrez leurs commentaires dans le carnet de travail pour que quelqu'un puisse les traiter ultérieurement. Si l' équipe estime que vos modifications ajoutent plus de risque ou de complexité que de valeur, vous devez résoudre les problèmes en conséquence.

Rappelez-vous simplement que tout code a au moins un cas supplémentaire pouvant être testé et pouvant utiliser au moins un refactoring supplémentaire. C'est pourquoi il est préférable de réviser le code en groupe, tout le monde cherchant le même code en même temps. Pour que tout le monde finisse par se mettre d'accord sur le point de savoir si le code en revue est acceptable (tel quel) et ajoute suffisamment de valeur pour se fondre dans la base de la communauté ou si certaines choses doivent être faites avant de pouvoir le faire. .

Puisque vous posez cette question, je suppose que vous ne faites pas réellement des "revues de code", mais que vous créez à la place une demande de tirage ou un autre mécanisme de soumission que les autres peuvent commenter de manière non déterministe. Alors maintenant, vous êtes dans un problème de gestion et une définition de done. J'imagine que votre direction est indécise et ne comprend pas vraiment le processus et le but des révisions de code et n'a probablement pas de "définition de fait" (DOD). Parce que s'ils le faisaient, votre DOD répondrait explicitement à cette question et vous n'auriez pas à venir ici et à vous le demander.

Comment corrigez-vous cela? Eh bien, demandez à votre responsable de vous donner un DOD et de vous indiquer si vous devez toujours mettre en œuvre tous les commentaires. Si la personne en question est votre responsable, la réponse est évidente.

Cette question ne porte pas sur les vertus de la programmation défensive, sur les dangers des cas de coin ou sur les risques catastrophiques de bugs dans les produits physiques. En fait, il ne s'agit même pas du tout d'ingénierie logicielle .

Il s’agit vraiment de savoir comment un pratiquant débutant traite une instruction donnée par un pratiquant âgé lorsque le subalterne ne peut pas l’accepter ni l’apprécier.

Il y a deux choses que vous devez apprécier en tant que développeur junior. Tout d’abord, cela signifie que même s’il est possible que vous ayez raison, et que vous avez tort, c’est - selon la prépondérance des probabilités - peu probable. Si votre collègue fait une suggestion dont vous ne voyez pas la valeur, vous devez envisager sérieusement la possibilité que vous n’êtes pas suffisamment expérimenté pour la comprendre. Je ne comprends pas ce message de ce post.

La deuxième chose à comprendre est que votre partenaire principal s'appelle ainsi parce qu'il a plus de responsabilités. Si un junior casse quelque chose d'important, il ne sera pas dérangé s'il suit les instructions. Toutefois, si une personne âgée leur permettait de le rompre - en ne soulevant pas de problèmes lors de la révision du code, par exemple -, elle aurait tout à fait raison de s'inquiéter.

En fin de compte, vous devez observer les instructions de ceux qui ont confiance dans l’entreprise pour mener leurs projets, ce qui est une exigence implicite de votre travail. Êtes-vous généralement incapable de laisser tomber les personnes âgées lorsqu'il existe de bonnes raisons de valoriser leur expérience? Avez-vous l'intention de ne suivre aucune instruction que vous ne comprenez pas? Pensez-vous que le monde devrait s'arrêter jusqu'à ce que vous soyez convaincu? Ces valeurs sont incompatibles avec le travail en équipe.

Un dernier point. Repensez aux projets que vous avez écrits il y a six mois. Pensez aux projets que vous avez écrits à l'université. Vous voyez à quel point ils semblent maintenant - tous les bugs et la conception à l'envers, les angles morts et les abstractions erronées? Et si je vous disais que dans six mois, vous aurez les mêmes défauts dans le travail que vous faites aujourd'hui? Cela vous aide-t-il à démontrer le nombre de angles morts dans votre approche actuelle? Parce que c'est la différence que l'expérience fait.

fait constamment des commentaires dans mon code qui me suggèrent de faire beaucoup plus de travail que nécessaire.

Vous pouvez donner des recommandations, mais au final, ce n’est pas à vous de décider de ce qui est nécessaire. Il vous appartient de mettre en œuvre ce que la direction ou (dans ce cas, votre relecteur) juge nécessaire. Si vous êtes en désaccord avec ce qui est trop ou trop nécessaire, vous vous retrouverez probablement au chômage. Par conséquent, votre professionnalisme consiste à accepter et à être en paix avec cela.

Il m'avait suggéré de gérer un cas obscur qui aurait très peu de chance de se produire

Il existe d’autres bonnes réponses ici qui montrent que même les non-bogues (c’est-à-dire quelque chose qui peut à coup sûr ne jamais échouer ) devraient toujours être retravaillés parfois. (par exemple, dans le cas d’une construction future de la sécurité du produit, du respect des normes, etc.) Le rôle d’un bon développeur est en partie d’avoir la plus grande confiance possible dans le fait que votre code sera robuste dans toutes les situations imaginables à chaque fois et dans le futur. également protégé, pas seulement dans des situations testées dans les conditions actuelles la plupart du temps

Suggestions aux réviseurs de code pour augmenter l'utilité commerciale de votre révision de code (vous, OP, devriez proposer un tel changement):

• Notez vos commentaires par type. "Critical" / "Must-Do" / "Facultatif" / "Améliorations suggérées" / "sympa d'avoir" / "Je réfléchis".

  Si cela vous semble trop CDO / anal / compliqué, utilisez au moins 2 niveaux: "Vous devez résoudre le problème pour passer le processus de révision et être autorisé à fusionner votre modification" / "Tous les autres".

Suggestions pour gérer les suggestions de révision de code qui semblent moins critiques à faire:

• Créez un ticket ouvert dans le système de ticket de votre choix (votre équipe en utilise un, si tout va bien?), En suivant la suggestion

• Placez le numéro de ticket en tant que commentaire de réponse à l'élément de révision de code si votre processus autorise les réponses aux commentaires tels que Fisheye ou les révisions par courrier électronique.

• Contactez le réviseur et demandez explicitement si cet élément est du type "doit être réparé ou ne sera pas fusionné / publié".

  • Si la réponse est "Oui", mais que vous n'êtes pas d'accord, laissez la personne responsable de la gestion du projet (MP, votre responsable, etc.) prendre une décision - présentez le désaccord de manière complète et honnête. Il ne s'agit pas de savoir qui de vous "a raison", mais de savoir ce qui est le mieux pour le projet, donc le travail de PM / manager.

Traitez maintenant ce ticket comme toute autre demande de développement.

1. S'il est décidé d'être urgent après l'escalade, traitez-le comme n'importe quelle demande de développeur urgente. Dépriorisez d'autres travaux et travaillez dessus.

2. Sinon, travaillez dessus en fonction de la priorité qui lui a été attribuée et de son retour sur investissement (qui peut varier en fonction de votre secteur d'activité, comme expliqué dans d'autres réponses).

Vous ne devriez pas en parler à la direction.

Dans la plupart des entreprises, le responsable de la gestion choisira toujours de ne pas écrire ce test supplémentaire, de ne pas perdre de temps à améliorer légèrement la qualité du code, de ne pas perdre de temps en refactorisation.

Dans de nombreux cas, la qualité du code dépend des règles non écrites de l'équipe de développement et de l'effort supplémentaire fourni par les programmeurs.

Vous êtes un développeur junior et ceci est votre première révision de code . Vous devez accepter les conseils et faire le travail. Vous ne pouvez améliorer le flux de travail et les règles de votre équipe que si vous les connaissez et que vous les respectez pendant un certain temps afin de pouvoir comprendre pourquoi elles sont présentes. Sinon, vous serez ce nouveau type qui ne respecte pas les règles et devient le loup solitaire de l'équipe.

Vous êtes nouveau dans l'équipe, suivez les conseils que vous recevez pendant un certain temps, découvrez pourquoi ils sont là, n'apportez pas le premier conseil que vous avez mis en question lors de la réunion Scrum. Les véritables priorités de votre entreprise vous apparaîtront au bout d’un moment sans demander rien (et ce n’est peut-être pas ce que le responsable de la gestion vous dira face à face).

Il est très important que votre code soit conforme à votre demande de prospect / gestion. Tout autre détail serait juste un "bon à avoir". Si vous êtes un expert (lisez: "si vous n'êtes pas un développeur junior") dans votre domaine, vous êtes alors "éligible" pour résoudre les problèmes mineurs que vous rencontrez en cours de route sans consulter vos dirigeants à chaque fois.

Si vous pensez que quelque chose ne va pas et que vous êtes relativement expert dans votre domaine, alors vous avez de bonnes chances d'avoir raison .

Voici quelques déclarations qui pourraient vous être utiles:

• On m'a demandé de faire X, le réviseur de code suggère également de faire Y, devrais-je le faire ou devrais-je passer à des tâches plus importantes?
• Vous me suggérez Y, alors pouvez-vous trouver au moins un scénario de test susceptible d’attraper ce comportement afin que je puisse le tester? Je crois que ce code ne sera pas appelé.
• Peut-être ne devrions-nous pas d'abord mettre en place une solution de secours sûre pour les cas non couverts? Donc, nous les attrapons tôt et nous nous débrouillons pour pouvoir nous concentrer sur des tâches plus importantes.
• OK, pendant que j'implémente Y, pourriez-vous avoir la gentillesse d'écrire quelques scénarios de test afin que nous puissions le faire une fois pour toutes ?
• On m'a demandé de faire X et je pense que je pourrais le faire sauf s'il y a d'autres priorités. La prochaine fois, pourquoi ne pas faire une demande de fonctionnalité au lieu de la mettre en commentaire de révision de mon code ? Au moins, nous pouvons entendre les avis d’autres membres de l’équipe sur cette fonctionnalité avant de la mettre en œuvre (généralement, tout élément important doit être une fonctionnalité et doit être géré par plus d’une personne; en général, la révision du code doit consister en une révision du code et des solutions proposées; le reste est la correction de bugs et fonctionnalités).

Pensez-vous sérieusement que l'attitude de votre critique endommage le projet ou pensez-vous qu'il a raison la plupart du temps (sauf qu'il fait parfois des erreurs d'évaluation mineures et l'exagère)?

Pour moi, cela ressemble plus à un texte qu’il écrit dans une révision de code, ce qui est une mauvaise pratique, car il est plus difficile pour tout le monde de faire un suivi. Cependant, je ne sais pas ce qu'il a dit dans d'autres critiques, je ne peux donc rien en dire.

En général, essayez d'éviter les deux problèmes suivants:

• Vous ne faites pas ce qui a été demandé
• Vous faites en sorte que votre critique se sente idiot

S'il examine réellement votre code, c'est parce que la direction lui fait davantage confiance que vous.

En cas de désaccord lors de la révision du code concernant la portée:

1. Documentez la portée réellement couverte par le code. Personne n'aime les mauvaises surprises.
2. Réaliser que la portée est une décision commerciale. L'étendue devrait déjà être connue au moment où vous commencez à travailler sur une fonctionnalité, mais si ce n'est pas le cas, vous pouvez toujours demander des éclaircissements plus tard.

Si le réviseur de code est celui qui prend les décisions commerciales, il peut modifier l'étendue à tout moment, même lors de la révision du code, mais il ne le fait pas dans son rôle de réviseur de code.

Si vous ne pouvez pas prouver que le cas de bord est impossible, vous devez le supposer possible. Si cela est possible, il est inévitable que cela se produise éventuellement, et le plus tôt possible. Si le cas d'extrémité ne s'est pas produit lors du test, cela peut indiquer que la couverture du test est incomplète.

1. Acceptez les commentaires.
2. Avant d'apporter des modifications à votre code, essayez de votre mieux pour créer un test pour le cas d'extrémité et voir si vous pouvez obtenir un échec de test (preuve de l'existence du problème). S'il est impossible de créer un cas de test de ce type et d'obtenir un échec de test, vous pouvez alors en conclure que le cas d'extrémité est en fait impossible (bien que j'hésiterais à en tirer une telle conclusion).
3. Si vous pouvez obtenir un échec de test, appliquez le changement de code approprié pour réussir le test.

Pour le bien du produit, vous voulez probablement être en mesure de produire le cas limite et de provoquer un échec, de manière à pouvoir appliquer le correctif et avoir la certitude qu'un problème potentiel a été évité.

Le but des revues de code est de placer des yeux supplémentaires sur le code. Aucun d'entre nous n'est à l'abri des erreurs ou des oublis. Il est tout à fait courant de regarder plusieurs fois un code et de ne pas remarquer une erreur évidente, où de nouveaux yeux peuvent le détecter immédiatement.

Comme vous l'avez dit, vous avez implémenté un nouvel algorithme. Il serait peu judicieux de tirer des conclusions sur le comportement de votre nouvel algorithme en fonction du comportement ou des observations concernant son prédécesseur.

Il y a des réviseurs de code qui savent ce qu'ils font et s'ils disent qu'il faut changer quelque chose, alors il faut que ce soit changé, et quand ils disent que quelque chose doit être testé, alors il faut que ce soit testé.

Il existe des réviseurs de code qui doivent justifier leur propre existence en créant un travail inutile pour les autres.

Laquelle est à vous de décider, et comment gérer le second type est plus une question de lieu de travail.stackexchange.

Si vous utilisez Scrum, alors la question est de savoir si votre travail fait ce qu’il est censé faire (apparemment, il le fait) et vous pouvez mettre le traitement du cas extrêmement rare et peut-être impossible dans l’arriéré, où il sera priorisé et si va dans un sprint, alors votre critique peut se sentir libre de le prendre et de faire les 13 heures de travail. Si vous faites le travail X et que vous réalisez que le travail Y a également besoin d'être accompli, le travail Y ne fait pas partie du travail X, il s'agit de son propre travail indépendant.