Quelle est l'importance des commentaires positifs dans les revues de code?

48

Est-il important de souligner les bonnes parties du code lors de son examen et les raisons pour lesquelles il est bon? Les commentaires positifs peuvent être tout aussi utiles pour le développeur examiné et pour les autres participants.

Nous effectuons des révisions à l'aide d'un outil en ligne, afin que les développeurs puissent ouvrir des révisions pour leur code validé, tandis que d'autres peuvent réviser leur code dans un délai donné (par exemple, une semaine). D'autres peuvent commenter le code ou les commentaires d'autres relecteurs.

Devrait-il y avoir un équilibre entre les commentaires positifs et négatifs?

c_maker
la source
3
Hé, si ça passe, c'est un retour positif. :)
Adrian J. Moreno
1
Dans une large mesure, je pense que cela dépend de la personne dont le code est en cours de révision. S'ils réagissent négativement lorsqu'ils ne reçoivent que des critiques, il est important de trouver un équilibre. sinon, les commentaires positifs sont redondants, car la révision réussie est intrinsèquement positive. S'ils font quelque chose de nouveau et de merveilleux, vous pouvez le mentionner, mais son intégration aux meilleures pratiques de votre équipe constituerait également un retour positif.
Matt
La SE inclut à la fois des votes positifs et des votes négatifs. Une rétroaction positive doit donc être importante pour que les choses fonctionnent correctement. Comment cela se passerait-il si le mieux que vos questions et réponses ici pouvaient espérer soit zéro? C'est une différence stéréotypée entre hommes et femmes: pour les hommes, aucun retour ne signifie "c'est bon". Pour les femmes, cela signifie: "il n'y avait rien de bon à dire". Cela pourrait très bien expliquer l'attrait relatif de ce domaine pour les hommes et les femmes.

Réponses:

41

Améliorez la qualité et le moral en utilisant des évaluations de code par des pairs
http://www.slideshare.net/SmartBear_Software/improve-quality-and-morale-using-peer-code-reviews

Ce que tout le monde devrait faire: Révision du code
http://scientopia.org/blogs/goodmath/2011/07/06/things-everyone-should-do-code-review/

Ces deux articles indiquent que l’un des objectifs de la révision du code est de partager les connaissances sur les bonnes techniques de développement, et pas seulement de rechercher les erreurs.

Donc, je dirais que c'est très important. Qui veut aller à une réunion et seulement être critiqué?

Robert Harvey
la source
26
Moi! Moi! Le sarcasme mis à part, je suis devenu très agacé par les revues de code où il n'y a aucune critique de mon code; Je sais que je n'ai pas fait les choses parfaitement et que le manque de critiques me donne l'impression de perdre mon temps, même en demandant une critique. Je conviens donc que rien que la critique n’est mauvais, mais aucun ne l’est aussi.
Jimmy Hoffa
3
J'ai tendance à être d'accord avec Jimmy Hoffa. En général (pas seulement dans les revues de code), je trouve très ennuyeux de traiter avec des personnes qui essaient de faire beaucoup de retours positifs. Un retour d'information positif devrait être utile: il n'est pas nécessaire de polluer l'examen en disant des choses que l'auteur du code sait déjà. Personnellement, je préfère l’attitude: "Tu es géniale et intelligente, mais il y a quelques problèmes mineurs dans ton code."
Arseni Mourzenko
6
@MainMa: " Ça a l'air bien" fonctionne pour moi, si aucun problème n'est trouvé. Pour un code particulièrement utile ou bien écrit: "Cela pourrait être utile. Mettons-le dans nos archives de partage de code avec quelques notes ou essayez de l'intégrer à nos pratiques de codage quotidiennes."
Robert Harvey
2
Nous avons déjà eu quelqu'un qui a arrêté de fumer à cause de l'horrible réputation de nos critiques de code. Nous avons depuis adopté l'utilisation de la révision de code en tant qu'atelier, avec quelques critiques de code pour les problèmes graves, mais surtout pour l'éducation. Le gars qui a démissionné s'est disputé avec notre manager lors de l'examen en raison de divergences d'opinions. Les gens peuvent être vraiment défensifs lors des examens, je recommande donc fortement de donner des commentaires positifs afin de dissiper les tensions et de faciliter les moments de réflexion "vous devriez changer cela" pour le commentaire, si ce n'est pour aucune autre raison que de vous casser de temps à autre
Brian
2
@Brian Je dois dire que si quelqu'un se dispute violemment avec une petite critique, il est probable qu'il nuit à la culture de l'entreprise et au moral des employés, je pense que vous êtes mieux loti.
Jimmy Hoffa
29

Quand je fais des revues de code, j’ai tendance à avoir un monologue en cours d’exécution, donc si je comprends ce que je lis, il y aura beaucoup de "Ok, je vois ce que ça fait .. Bon, ça relie à ça et appelle ça, ça va… et cette pièce dépend de ces deux-là ça va. ".

Je pense que de cette façon, ce n'est pas "oo la la c'est tellement génial!", Cela pourrait être un code ennuyeux parfaitement trivial, mais entendre quelqu'un d'autre analyser et montrer que la compréhension de ce que vous avez écrit est une forme de rétroaction positive en soi, le retour étant "Ce code a du sens", lorsque je tombe sur des parties que je ne comprends pas, je demande des explications et lorsque je le comprends, je m'écrie "Ah, je l'ai eu".

Je pense que ce simple transfert de compréhension est un éloge à un autre ingénieur car nous souhaitons tous que notre code soit compris par d'autres, cela donne une forme de validation implicite.

Cela dit, si vous voyez des parties du code qui ont des caractéristiques bonnes ou positives (même un code trivial ennuyeux peut être bon s'il s'agit de la forme minimale d'elle-même), j'ai tendance à énoncer ces caractéristiques, encore une fois, je ne les attribue pas comme "Wow!" génial!" autant que "je vois que ceci est une implémentation minimale" ou "Ok, cet algorithme complexe a beaucoup de commentaires", concentrez-vous sur les attributs du code et non sur le bonté ou le mal inhérent.

Chaque fois que vous attribuez le "bien" ou le "mal" à un code lors d'une révision de code pour éviter que l'ingénieur ne se sente mépris ou tenu sur un socle, ne dites pas que quelque chose est bon ou mauvais, mais parlez plutôt de la cause et de l'effet de leur code.

"Ok cette partie a du sens, ah il y a un nombre magique ici, le sens de cette valeur pourrait ne pas être bien compris par le prochain ingénieur qui toucherait cela"

"Je vois que vous avez un conteneur DI ici ok donc vous aurez un couplage lâche avec ce référentiel"

"Ah, il y a un dictionnaire statique ici, si plusieurs threads le touchent, nous pourrions rencontrer certaines conditions de concurrence"

Remarquez, je ne dis pas que tout soit bon ou mauvais, mais si l'ingénieur doit le changer ou non, il sera compris par l'ingénieur dont le code est en cours de révision. Évidemment, vous devez mettre fin à la révision du code avec un yay ou un nay, mais accumuler ces déclarations au fil de celle-ci adoucira les nay car des explications ont déjà été données sous la forme de déclarations de cause à effet lorsque vous leur dites: "J'aimerais bien ces nombres magiques fixés avant de vérifier ceci dans ".

Jimmy Hoffa
la source
4
+1 pour "un simple transfert de compréhension est un éloge à un autre ingénieur ... cela donne une forme de validation implicite"
Roy Tinker
Je veux +1 ce deux fois. Un pour la même raison que @RoyTinker, et un pour "Ne dites pas que quelque chose est bon ou mauvais, mais parlez plutôt de la cause et de l'effet". Les deux très bons points!
Ben Lee
7

Si je trouvais quelque chose dans une revue de code qui me plaisait vraiment et qui dépassait largement le code "assez bon", je donnerais des commentaires positifs.

En général, je pense que si quelqu'un écrit un code d'article qui vous fait dire "Waouh, c'est vraiment sympa!" alors oui, les commentaires positifs sont importants - cela montre à l'auteur que ce qu'ils ont fait a plu aux autres, et qu'ils devraient essayer de le faire à nouveau. Cependant, il ne suffit pas de suivre les directives et les pratiques standard. Donner des éloges parce que quelqu'un a bien mis en retrait ou ajouté des commentaires passe-partout pourrait placer la barre assez bas.

FrustratedWithFormsDesigner
la source
6

Ce n’est pas tant une question de programmation que c’est une question de gestion générale et d’interactions humaines. Les commentaires positifs dans les revues de code sont tout aussi importants que les commentaires positifs dans toutes sortes de revues.

Que cela soit nécessaire ou non (et dans quelle mesure), cela dépend de la disposition et de la composition émotionnelle de la personne à qui vous parlez. Certaines personnes réagissent beaucoup plus efficacement aux corrections lorsqu'elles sont associées à des éloges. D'autres voient les éloges comme peu sincères lorsqu'ils sont livrés avec correction.

La formule générale est parfois appelée "Feedback Sandwich": les bonnes choses en premier, les mauvaises choses en second, les bonnes choses en dernier. L'idée est de garder le ton général positif tout en veillant à ce que les commentaires négatifs soient reçus. Cela peut aider à prévenir le stress lors de l’anticipation d’un examen et à prévenir les rumeurs égocentriques par la suite. Les deux sont très importants en termes de productivité et de qualité. Ce n’est pas simplement une foutaise délicate et émotionnelle; C'est le comportement humain 101.

Encore une fois, vous devez connaître la personne avec laquelle vous travaillez et comprendre à quoi elle répond. Traiter avec les gens, c'est ça la gestion, et les bons gestionnaires savent comment faire réagir les gens.

tylerl
la source
4

Je pense que le retour d'information positif est très important et provient principalement d'une dynamique personnelle de realpolitik. Nous sommes tous assis et écrivons du code pendant des heures, des jours, des semaines, des mois et la plupart d'entre nous sommes fiers de ce que nous faisons. Les revues de code sont une chance de montrer cela.

Si vous passez à une révision de code et que le meilleur résultat que vous pouvez espérer est "pas de commentaire" (c.-à-d. Qu'il n'y a pas de bilan de réactions positives), la réunion pourrait facilement être intitulée dans "Découvrez comment les gens vous pensent mal, c'est mal". En conséquence, les développeurs vont commencer à être ennuyés par les critiques de code, voire à les craindre, ce qui nuit clairement à l’équipe. Les développeurs "oublieront" de faire réviser leur code ou développeront leur impuissance acquise et demanderont simplement à leurs critiques constants quoi faire à propos de tout pour ne pas se laisser abattre lors de ces réunions.

C'est bien beau de dire qu'en théorie, il est plus logique de réparer le mauvais et de demander à tout le monde de laisser les émotions à la porte, mais ce sont précisément des attitudes comme celle-là qui sont responsables pour les développeurs de représentants comme étant sourds sur le ton interpersonnel. Les théories mises à part, nous sommes des êtres humains et les êtres humains aiment recevoir une tape dans le dos de temps en temps, même minime. Ce truc compte.

Erik Dietrich
la source
Cela me rappelle le concept appelé "Appreciative Inquiry", qui cherche à améliorer et à développer ce qui fonctionne déjà, plutôt que de se concentrer sur les problèmes à résoudre.
3

C’est plus important si vous effectuez des examens côte à côte ou en équipe. Dans une critique écrite, pas de nouvelles, bonnes nouvelles. L'objectif est de faire entrer le code en production. Quand c'est votre code, vous devriez vous sentir bien dans votre peau.

La révision du code doit être utilisée comme source d'informations pour faciliter le mentorat et la gestion de l'équipe. Il existe de nombreuses possibilités de donner une rétroaction positive sans encombrer la base de données de révision de code. Des exemples peuvent être extraits pour être partagés avec d'autres.

Examiner le développeur autrement que son code représente beaucoup plus. Détourner le temps de révision du code peut être contre-productif pour faire sortir une application. Définissez un délai spécifique pour aider le développeur en dehors des révisions de code, mais cela ne signifie pas pour autant que vous devez exclure les commentaires relatifs à la révision de code.

JeffO
la source
2

Si je ne fais pas attention à éviter le "compliment de revers", le seul moyen auquel je puisse penser est de pouvoir vous donner un retour positif sur le code. La plupart des gens sont au courant de cela ... cela se traduit par des phrases comme: "Excellent travail, mais ..."

Si tout le monde vient à la réunion avec l’attitude qu'il ne s’agit pas d’un commentaire personnel du programmeur, mais bien d’un effort visant à améliorer les pratiques de codage pour la qualité de tout le système, tous les commentaires sont alors de "bons" commentaires. Les commentaires soulignant les moyens d'améliorer les pratiques de codage deviennent tout aussi importants que les commentaires soulignant une nouvelle méthode utile pour traiter un problème.

À tout le moins, si on ne va pas aussi loin, il faut souligner que le cycle de "bon retour, mauvais retour, bon retour, mauvais retour" dans le processus de révision même sentiment de compliment de revers. N'essayez pas de forcer une bonne rétroaction, essayez de renforcer les bons efforts et comblez des trous dans les connaissances.

Phrases dont j'ai le plus appris au fil des ans:

  • "C'est une approche intéressante. Que se passe-t-il si nous devons prendre en compte [un autre cas d'utilisation]?"
  • "Bien essayé! Saviez-vous que nous avons déjà une méthode pour le faire? Peut-être devrions-nous faire une analyse comparative pour voir quelle approche est la plus efficace."
Jason M. Batchelor
la source
2

Le workflow que j'ai le plus aimé avec les critiques de code était le suivant:

  1. Dev soumet le correctif sur la liste de diffusion / l'outil en ligne.
  2. Tous ceux qui ont besoin de soins s’intéressent au patch, suggèrent des améliorations.
  3. Dev remonte à # 1
  4. Si aucune amélioration n'est requise, les gens disent: "Bon travail, s'il vous plaît, engagez-vous". <- RETOUR POSITIF. Tout code accepté est bon. Moins les gens doivent vous dire de changer les choses, mieux vous vous portez.
  5. Dev s'engage, passe à l'élément suivant.

Habituellement, les nouveaux développeurs obtiendraient beaucoup plus de commentaires «correctionnels» à mesure qu'ils se familiariseraient avec le code.

Les avantages de cette approche sont les suivants:

  1. Tout le monde sait ce que tout le monde fait. Il n'y a pas de connaissance monopolisant ou mystère commet.
  2. Tout le monde apprend des commentaires des autres. C'est important. Si les réactions ne se produisent qu'entre deux personnes dans une conversation face à face lors d'un jumelage, alors quelqu'un de l'autre côté de la salle ne bénéficiera pas de la même manière que s'il le faisait sur la liste de diffusion.
  3. Les autres développeurs peuvent généralement détecter certains bogues avant qu'ils ne soient dans le contrôle de version.
MrFox
la source
Donc, fondamentalement, vous espérez ne pas avoir de retour du tout. Pourquoi se donner la peine d'aller au travail? Vous pouvez être invisible à la maison.
1

Je ne suis pas du tout d'accord avec ça. Quelle est la différence entre les bonnes techniques de développement et les soi-disant codeurs Ninja capables d'écrire un code impressionnant mais inexplicable en simple mortel? Le développement logiciel est maintenant (IMO) une discipline du plus petit dénominateur commun où le flair et la ruse sont rejetés au profit de la facilité de maintenance et de la facilité de compréhension. C'est trop risqué.

Je ne peux pas penser à une fois où j'ai vu du code dans une critique qui me ferait dire «Oh c'est cool». Je ne peux que supposer que si je rencontrais un code comme celui-ci, il tomberait dans le camp Cool-Yet-Inacceptable.

Vous auriez également des problèmes avec les personnes qui n’obtiennent aucune réaction positive, peut-être même trop, en essayant de créer un gâchis éventuel "Faites-moi confiance, ça marche!".

Les revues de code servent à répartir la responsabilité de la qualité du code entre les membres de l’équipe, c’est-à-dire qu’un développeur ne peut être blâmé si un problème sérieux se pose plus tard. Utilisez-le pour trouver des problèmes, utilisez-le pour obtenir des explications du développeur original d'éléments étranges au cas où vous auriez à les entretenir. Personnellement, je suis plus intéressé à recevoir des commentaires négatifs. Les clients ne se soucient pas de la fraîcheur de votre code, mais seulement de ce qu'il fait ce qu'ils veulent.

Laissez le backslapping au pub.

James
la source
1
"Les clients ne se soucient pas de la fraîcheur de votre code, seulement qu'il fasse ce qu'ils veulent." Les clients ne s'intéressent pas non plus à la lisibilité du code. Ils se soucient peut- être du temps qu’il faut pour réparer un bogue, ajouter une fonctionnalité ou modifier un comportement, mais ils ne se soucient certainement pas de la lisibilité du code en soi.
un CVn
1

Cela a compté pour moi. Je ne veux pas de commentaires de fluff ou de positivité pour des raisons de positivité. Si tout le code que j'ai écrit est de la merde, dis-moi pourquoi, corrigeons-le et apprenons. Mais si je fais quelque chose de bien, il est agréable de l'entendre une fois et de temps en temps. Je n'ai pas besoin de renforcement positif car tout ce que j'ai fait était "correct", mais même s'il s'agit d'un "améliorons X, Y et Z, mais le reste a l'air bien", c'est important.

Peacedog
la source
0

Pas aussi important que les commentaires honnêtes. Je travaille pour une grande société financière et nos clients ne se soucient pas de savoir si le programmeur fait de gros efforts ou s'il est bon, ou s'il écrit un bon code! Ils ont besoin d'un logiciel qui fonctionne.

Aserwin
la source
D'après mon expérience, les programmeurs qui font de gros efforts, qui sont bons et qui sont satisfaits d'une équipe de soutien ont tendance à écrire un logiciel qui fonctionne.
c_maker
Poulet et oeuf je suppose. Mais la question portait sur la révision du code ... ce qui, à mon avis, n’est pas le moment de se
laisser aller
L'examen du code n'est pas le moment de déterminer si les parties du logiciel visibles par l'utilisateur fonctionnent conformément aux spécifications.
un CVn
0

Je pense qu'il est important d'être complètement objectif. Vouloir remonter le moral en faisant des commentaires positifs est une perte de temps pour moi.

Cela peut signifier que les critiques de code sont indûment critiques - mais n'est-ce pas là le problème? Nous devrions également être critiques de nous-mêmes. Je trouve que l’hypothèse que le code que j’ai écrit est sans doute une ordure complète et qu’elle pourrait certainement être améliorée me pousse à améliorer mon code et mon niveau de compétence.

Si vous n'obtenez aucun commentaire, vous pouvez considérer que vous avez fait du bon travail.

utilisateur619818
la source
C'est peut-être pourquoi la plupart des programmeurs sont des hommes.
-1

Le mantra est simple: si on veut un code de qualité (qui est moins en réalité), il faut mettre en pratique les méthodes de révision appropriées. Cela dit, les commentaires positifs aident un développeur / programmeur à réfléchir et à proposer des idées / solutions / correctifs. Ne soyez pas trop sévère, mais soyez ferme sur le point. Questions / Réponses Les responsables doivent être informés des bonnes méthodologies et pratiques afin de pouvoir guider l'équipe (ou un membre) dans la bonne direction. Cela se traduit par la qualité. Période.

Akula
la source
-3

Lorsque le code est destiné à un concours ou soumis à un entretien d'embauche (en d'autres termes, un code écrit qui ne peut pas être réécrit), les commentaires positifs sont indispensables. En fait, vous devez vous assurer qu'il y a des retours positifs (si possible!) Et négatifs. De cette façon, le codeur sait où se trouvent ses forces et ses faiblesses et peut compenser.

Cependant, vous semblez parler dans un environnement de travail, où le code peut être réécrit. Dans ce cas, vous essayez de récupérer des bugs de votre système. Ainsi, dans cette situation, seuls les bugs négatifs ont une valeur.

Si cela vous met mal à l'aise, organisez une réunion hebdomadaire de révision du code, où tout le monde peut discuter à la fois du bon et du mauvais code.

EDIT: Bien que je dirai que si quelque chose vous impressionne assez, rien ne vous empêche d’exprimer vos louanges en personne. Le traqueur semble cependant ne servir qu’à la révision du code de production.

KBKarma
la source
6
"Donc, dans cette situation, seuls les bugs négatifs ont une valeur." - Je ne suis pas du tout d'accord avec ça. Si quelqu'un trouve un excellent moyen de corriger un bogue / d'implémenter une fonctionnalité, ce n'est absolument pas inutile. Et maintenir la motivation est important. Si vous ne faites que souligner l'échec, vous allez avoir des problèmes.
Mat
Mauvais choix de mots de ma part. Bien que les bonnes choses ne valent rien (avoir écrit assez de scories à mon époque), les bugs sont, très probablement, ce pour quoi le traqueur a été configuré. Le PO peut, s’il le souhaite, laisser des commentaires positifs. Mais personnellement, je laisserais ça pour des conversations en face à face, car cela éviterait que le traqueur ne soit bouché. En outre, je suis très ennuyé je ne peux pas voter des commentaires. :)
KBKarma
1
@KBKarma - si vous estimez que votre réponse d'origine n'a pas été formulée aussi bien qu'elle aurait pu l'être, veuillez revenir en arrière et éditer votre réponse pour qu'elle reflète correctement vos pensées. Recherchez le bouton Modifier situé sous votre réponse.