Pratiques agiles: révision du code - Échec de la révision ou problème?

53

À la fin d'un sprint de 2 semaines et d'une tâche comportant une révision du code, nous découvrons dans cette revue une fonction qui fonctionne, qui est lisible, mais qui est assez longue et qui a quelques odeurs de code. Travail de refactor facile.

Sinon, la tâche correspond à la définition de done.

Nous avons deux choix.

  • Échouez à la révision du code, de sorte que le ticket ne ferme pas dans ce sprint, et nous sommes un peu gênés par le moral, car nous ne pouvons pas le passer.
  • Le refactor est un petit travail qui devrait être fait lors du prochain sprint (ou même avant qu'il ne commence) sous la forme d'une histoire minuscule à demi-point.

Ma question est la suivante: existe-t-il des problèmes ou des considérations inhérents au fait de lever un ticket à l’arrière d’une revue au lieu de l’échouer?

Les ressources que je peux trouver et lire des critiques de code détaillé en tant que 100% ou rien, généralement, mais je trouve que ce n’est généralement pas réaliste.

Erdrik Ironrose
la source
23
Donc, si vous ne pouvez pas échouer à la révision du code pour cela, quel est le but de la révision? À l'heure actuelle, il vous semble que c'est pour voir si quelque chose fonctionne , cependant, c'est sûrement le travail d'un test ou d'un testeur, pas d'une révision de code.
VLAZ
21
Je pense que la plupart des réponses manquent un point important dans votre question: sinon, la tâche correspond à la définition de fini. Les problèmes que vous mentionnez font-ils partie de ce que votre équipe considère comme une tâche "non terminée"? Ou ces questions ne sont pas prises en compte dans ce qu'une tâche "fait" devrait être? Si votre définition de «terminé» comprend «aucun code ne sent», la tâche n'est tout simplement pas terminée.
Josh Part
1
@ErdrikIronrose semble donc que le changement n'était pas conforme aux normes et n'était peut-être pas (aussi facilement) maintenable. Bien que votre autre commentaire semble indiquer que le changement ne faisait pas partie de la demande, dans ce cas, il ne devrait pas faire partie de la révision du code. Si quelqu'un écrit un code correct et conforme aux normes à côté d'un piratage laid existant, n'hésitez pas, créez un ticket pour réparer ce piratage laid et passez la révision de code actuelle. Si quelqu'un écrit un code correct, mais pas conforme aux normes (comme votre question l'indique), n'effectuez pas la révision du code tant qu'il n'a pas été correctement exécuté.
VLAZ
9
@ErdrikIronrose: Ah, donc l'odeur de code n'a pas été créée pendant que vous travaillez sur l'histoire à l'étude , mais existait déjà? C'est une distinction importante - envisagez de modifier la question.
Sleske
1
@vlaz vous devriez faire une réponse à partir de votre commentaire
Ister

Réponses:

67

Existe-t-il des problèmes ou des considérations inhérents à la création d'un ticket à l'arrière d'une revue au lieu de l'échec?

Pas intrinsèquement. Par exemple, la mise en œuvre du changement actuel a peut-être mis au jour un problème qui existait déjà, mais qui n'était pas connu / apparent jusqu'à présent. Échouer le ticket serait injuste, car vous échoueriez pour quelque chose qui n’était pas lié à la tâche décrite.

dans la critique on découvre une fonction

Cependant, je suppose que la fonction ici est quelque chose qui a été ajouté par le changement actuel. Dans ce cas, le ticket doit échouer car le code n'a pas réussi le test olfactif.

Où pourriez-vous tracer la ligne, sinon où vous l'avez déjà tracée? Vous pensez clairement que ce code n'est pas suffisamment propre pour rester dans la base de code dans sa forme actuelle; alors pourquoi envisageriez-vous de donner un ticket au billet?

Échouez à la révision du code, de sorte que le ticket ne ferme pas dans ce sprint, et nous sommes un peu gênés par le moral, car nous ne pouvons pas le passer.

Il me semble que vous dites indirectement que vous essayez de donner à ce ticket un laissez-passer pour améliorer le moral de l'équipe, plutôt que pour la qualité de la base de code.

Si tel est le cas, vos priorités sont partagées. La norme de code propre ne doit pas être modifiée simplement parce que cela rend l'équipe plus heureuse. La justesse et la propreté du code ne dépendent pas de l'humeur de l'équipe.

Le refactor est un petit travail qui devrait être fait lors du prochain sprint (ou même avant qu'il ne commence) sous la forme d'une histoire minuscule à demi-point.

Si la mise en œuvre du ticket d'origine a provoqué une odeur de code, il doit être traité dans le ticket d'origine. Vous ne devriez créer un nouveau ticket que si l'odeur du code ne peut pas être directement attribuée au ticket d'origine (par exemple, un scénario "paille qui casse le dos du chameau").

Les ressources que je peux trouver et lire des critiques de code détaillé en tant que 100% ou rien, généralement, mais je trouve que ce n’est généralement pas réaliste.

Réussir / échouer est intrinsèquement un état binaire , qui est intrinsèquement tout ou rien.

Je pense que ce dont vous parlez ici, c’est plus que vous interprétez les révisions de code comme exigeant un code parfait ou autrement échouant, et ce n’est pas le cas.

Le code ne doit pas être impeccable, il doit simplement respecter les normes de propreté raisonnables que votre équipe / entreprise emploie. L’adhésion à cette norme est un choix binaire: elle adhère (passe) ou pas (échoue).

D'après votre description du problème, il est clair que cela ne correspond pas à la norme de code attendue et qu'il ne devrait donc pas être transmis pour des raisons ultérieures telles que le moral de l'équipe.

Sinon, la tâche correspond à la définition de done.

Si "le travail était fait" était la meilleure référence en termes de qualité de code, nous n'aurions pas dû inventer le principe de code propre et de bonnes pratiques: le compilateur et les tests unitaires seraient déjà notre processus de révision automatisée. vous n'auriez pas besoin de critiques de code ou d'arguments de style.

Flater
la source
26
"L'exactitude et la propreté du code ne dépendent pas de l'humeur de l'équipe." +1 pour cela seul, cependant, la seule réserve à cette réponse serait de respecter un délai. Si l'échec de cette révision de code signifie qu'une fonctionnalité très attendue ne sera pas intégrée à la prochaine version, vous devez équilibrer la propreté du code avec les besoins du client. Mais rappelez-vous qu'un code incorrect qui respecte la date limite du client aujourd'hui est un problème de production demain.
Greg Burghardt
11
Grande réponse - ferme mais pas impoli. Un point tangentiel peut également être: comment avons-nous pu effectuer des révisions de code si tard dans le sprint qu'un refactor facile ne pourrait pas être fait sans provoquer l'échec de tout le sprint?
Daniel
@ Daniel: Le développeur peut être engagé d'une autre manière, ou il peut s'agir d'un problème de planification. Le temps entre la fin d'une tâche et la fin du sprint est généralement minime car (dans un monde idéal), les personnes achèveraient leur dernière tâche du sprint vers la fin du sprint. Vous ne pouvez pas prendre une période prolongée pour réviser / réparer; ou bien, peut-être que le développeur n'est tout simplement pas présent / disponible pour le reste du sprint.
Flater
8
+1 Les programmeurs peuvent se sentir bien lorsqu'ils ont écrit un bon code. Ignorer votre contrôle de la qualité n’est pas la solution pour améliorer votre moral. Un rejet occasionnel pour des problèmes mineurs n’est pas de nature à faire souffrir le moral, de toute façon. Si votre moral souffre du fait que vous échouez régulièrement au contrôle de la qualité, la solution consiste à remédier au problème de l'échec du contrôle de la qualité, sans abandonner les normes.
jpmc26
1
@GregBurghardt: Parce que de nombreuses entreprises ont abusé de l'argument de délai, j'ai tendance à n'accepter une mauvaise critique que si et seulement si une tâche pour sa refactorisation immédiate est créée et planifiée pour le premier sprint post-release. Le coût en temps supplémentaire ajoute une barrière d'entrée significative pour contourner la qualité du code.
Flater
38

À la fin d'un sprint de 2 semaines, une tâche comporte une révision de code [...] Travail de refactorisation facile.

Pourquoi cela apparaît-il à la fin du sprint? Une révision du code devrait avoir lieu dès que vous pensez que le code est terminé (ou même avant). Vous devriez vérifier votre définition de fini avec chaque histoire que vous terminez.

Si vous vous retrouvez à finir des histoires si peu de temps avant votre revue de démo / sprint que vous ne pouvez pas lui attribuer une "petite" tâche, vous devez améliorer votre estimation du travail. Oui, cette histoire ne s'est pas terminée. Pas à cause d'une révision de code, mais parce que vous n'aviez pas prévu d'intégrer les modifications apportées par la révision de code. C'est comme si on estimait que "tester" ne prenait pas de temps, parce que "si vous l'avez programmé correctement, cela fonctionnera, n'est-ce pas?". Cela ne fonctionne pas comme ça. Les tests trouveront des erreurs et la révision du code trouvera des choses à changer. Sinon, ce serait une grosse perte de temps.

Donc, pour résumer: oui, le DoD est binaire. Réussir ou échouer. Une révision de code n'est pas binaire, elle devrait plutôt ressembler à une tâche en cours. Vous ne pouvez pas échouer . C'est un processus et à la fin c'est fait. Mais si vous ne planifiez pas correctement, vous n'atteindrez pas cette étape "terminée" dans le temps et resterez coincé dans le territoire "non terminé" à la fin du sprint. Ce n'est pas bon pour le moral, mais vous devez en tenir compte dans la planification.

nvoigt
la source
5
C'est exactement la réponse qui m'est venue à l'esprit. Si chaque histoire est mise en œuvre avec sa propre branche, ne différez pas la révision et la fusion des branches jusqu'à la fin du sprint. Au lieu de cela, créez une demande d'extraction dès que la branche est supposée être prête et continuez à l'itérer jusqu'à ce qu'elle soit terminée, approuvée et fusionnée. Si ce processus n'a pas fini à la fin du sprint, l'histoire n'est pas terminée.
Daniel Pryden
20

Simple: vous passez en revue le changement . Vous ne révisez pas l'état du programme autrement. Si je corrige un bogue dans une fonction de 3 000 lignes, vous vérifiez que mes modifications corrigent le bogue, et c'est tout. Et si ma modification corrige le bogue, vous acceptez la modification.

Si vous pensez que la fonction est trop longue, vous introduisez une demande de modification pour la raccourcir ou la scindez-la une fois que ma modification a été acceptée. Cette demande de modification peut ensuite être hiérarchisée en fonction de son importance. Si la décision est prise que l'équipe a des tâches plus importantes à faire, cela est traité plus tard.

Il serait ridicule que vous décidiez des priorités de développement lors de la révision du code, et le rejet de mon changement pour cette raison serait une tentative de décider des priorités de développement.

En résumé, il est absolument acceptable d'accepter un changement de code et de générer immédiatement un ticket en fonction de ce que vous avez vu lors de l'examen du changement. Dans certains cas, vous le ferez même si le changement lui-même a causé les problèmes: S'il est plus important d'avoir les changements maintenant que de les résoudre. Par exemple, si d'autres personnes ont été bloquées en attendant le changement, vous souhaitez les débloquer pendant que le code peut être amélioré.

gnasher729
la source
4
Je pense que dans ce cas, le changement a été la fonction trop longue - si vous avez introduit une fonction de 3000 lignes qui n'existait pas auparavant (ou une fonction de 10 lignes auparavant).
user3067860
3
En principe, cette réponse est tout à fait correcte. Dans la pratique ..... Si tous les développeurs croient en et appliquent de bonnes pratiques de codage équilibrées par rapport aux efforts fournis, vous ne rencontrerez probablement pas ce problème très souvent et cette réponse sera alors nette. Cependant ... il semble qu'il y ait toujours un ou deux développeurs qui font tout ce qui est rapide pour faire gagner 5 minutes maintenant; alors qu’ils ignorent les heures, les jours ou les mois qu’ils ajoutent à un travail plus tardif. Dans ces cas, cette solution n’est qu’une pente glissante, car il faut recommencer et redéfinir l’ensemble du système.
Dunk
+1, mais je pense que vous devriez reformuler le dernier paragraphe pour faire ressortir que la vérification de code avec des problèmes devrait être une exception absolue. Je veux dire, juste que quelqu'un soit bloqué n'est pas une excuse suffisante. Ne pas réussir un seul sprint ne semble pas non plus être une excuse suffisante, et certainement pas une excuse qui pourrait être utilisée à plusieurs reprises.
Frax
@ user3067860 Si vous avez transformé une fonction à 10 lignes en une fonction à 3000 lignes, l'échec est évident. Si vous avez transformé une fonction de ligne 3000 en 3010 - alors passez probablement. Mais que se passe-t-il si vous avez transformé une fonction de 100 lignes (généralement un peu trop grosse) en une fonction de 300 lignes ( vraiment trop grosse)?
Martin Bonner soutient Monica
9

Échouez à la révision du code, de sorte que le ticket ne ferme pas dans ce sprint, et nous sommes un peu gênés par le moral, car nous ne pouvons pas le passer.

Cela semble être le problème.
En théorie, vous savez ce que vous devez faire, mais le délai est presque écoulé, vous ne voulez donc pas faire ce que vous savez devoir faire.

La réponse est simple: faites ce que vous feriez si vous obteniez le même code pour la révision du code le premier jour du sprint. Si ce serait acceptable, alors il le devrait maintenant. Si ce n'était pas le cas, ce ne serait pas maintenant.

xyious
la source
"Cher client, vous ne pouvez pas utiliser votre fonction avant 2 à 3 semaines car notre code a fonctionné, mais nous n'aimions pas son apparence", !
RandomUs1r
6
Les clients de @ RandomUs1r ne devraient pas avoir ce type d'informations. Cela n'a pas été fait parce qu'il n'y avait pas assez de temps pour ça et c'est tout. Les clients dictent-ils comment le code doit être écrit? Si vous appelez un électricien pour réparer le câblage de votre domicile, dites-vous "Changez simplement les câbles mais ne vous inquiétez pas de vérifier si ce sont les câbles appropriés"? Ou dites-vous à votre médecin "Je suis malade - donnez-moi des médicaments mais ne me diagnostiquez pas d'abord"? Les révisions de code doivent faire partie intégrante du travail et non pas selon les souhaits du client.
VLAZ
1
@ RandomUs1r: « »? Cher développeur, pourquoi pas la fonction terminée « - la réponse devrait être « parce que nous n'avons pas eu assez de temps pour le construire à un niveau de qualité acceptable », peut - être suivie » Nous pouvons lui donner à vous si vous êtes prêt à faire des compromis sur la qualité ".
Bryan Oakley
1
@ RandomUs1r vous devez donc en principe sacrifier la qualité du code, ce qui rendra probablement beaucoup plus difficile l'implémentation ultérieure des fonctionnalités. Un correctif de 2 jours maintenant pourrait très bien vous faire économiser un correctif de 4 semaines plus tard. C’est donc "Cher client, votre fonctionnalité ne peut pas être disponible avant 2 à 3 semaines car il faut tant de temps pour mettre en oeuvre une fonctionnalité mineure maintenant". Aussi, est-ce la fin d'un sprint ou est-ce une échéance majeure? S'il s'agit d'un délai important, je pourrais voir fusionner maintenant, écrire un correctif au cours des deux prochains jours et générer un PR juste après le délai.
xyious
5
Tout ce que je dis, c'est que si vos standards sont différents le premier jour et le dernier jour du sprint, vous n'avez alors aucun standard et votre qualité va inévitablement se perdre.
xyious
5

Une énorme partie du processus consiste à décider de ce que l'on veut faire et à s'en tenir à vos armes. Cela signifie également qu'il ne faut pas trop s'engager et que vos examens par les pairs sont terminés à temps pour permettre aux tests de s'assurer que le travail est également terminé sur le plan fonctionnel.

En ce qui concerne les problèmes de révision de code, il existe plusieurs façons de le gérer, et le bon choix dépend de plusieurs facteurs.

  • Vous pouvez simplement nettoyer le code vous-même et laisser la personne savoir ce que vous avez fait. Fournit quelques opportunités de mentorat, mais cela devrait être un travail assez simple qui peut être fait en quelques minutes.
  • Vous pouvez le renvoies avec des commentaires quant à ce qui est faux. Si la gestion des erreurs est mal effectuée ou si le développeur répète sans cesse les mêmes erreurs, cela peut être garanti.
  • Vous pouvez créer un ticket et contracter une dette technique. Le billet est là pour vous assurer de le rembourser plus tard. Il se peut que vous soyez pressé par le temps et que vous examiniez les changements, vous constatiez un problème plus grave qui n’était pas directement lié au changement.

En bout de ligne, c’est que lorsque vous avez terminé votre travail, vous devez le faire. S'il existe des problèmes plus importants que ceux sur lesquels le développeur a travaillé, placez le drapeau et avancez. Mais vous ne devriez pas être dans une position où il reste des heures avant la fin du sprint et que vous êtes en train de passer à l'examen par les pairs. Cela sent comme de trop engager vos ressources ou de tergiverser lors des examens par les pairs. (une odeur de processus).

Berin Loritsch
la source
4

La dépriorisation de problèmes liés à la révision du code ne présente aucun problème inhérent, mais il semble que les principaux problèmes sur lesquels vous devez vous mettre d'accord, en tant qu'équipe, sont les suivants:

  1. Quel est le but de votre révision de code?
  2. Quel est le lien entre les résultats de la révision du code et la définition de fait pour un élément de travail?
  3. Si la révision de code s'applique comme test de déclenchement, quels problèmes sont considérés comme des "bloqueurs"?

Tout cela revient à ce que l’équipe a accepté comme définition de Terminé. Si la révision de code avec zéro problème est la définition de terminé pour un élément de travail, vous ne pouvez pas fermer un élément qui n'a pas satisfait à cette exigence.

C'est la même chose que si, lors du test d'unité, un test d'unité avait échoué. Vous pouvez corriger le bogue et non ignorer le test unitaire si la réussite des tests unitaires est une condition requise pour être fait.

Si l'équipe n'a pas accepté que les révisions de code constituent une définition de Terminé, vos révisions de code ne constituent pas un test d'acceptation par déclenchement du work item. Il s’agit d’une activité d’équipe faisant partie de votre processus d’arriéré visant à rechercher du travail supplémentaire éventuellement nécessaire. Dans ce cas, les problèmes que vous découvrez ne sont pas liés aux exigences de l'élément de travail d'origine et constituent de nouveaux éléments de travail que l'équipe doit hiérarchiser.

Par exemple, il peut être tout à fait acceptable pour une équipe de ne pas hiérarchiser la correction des fautes de frappe dans les noms de variables car cela n’affecte pas la fonctionnalité métier livrée, bien que l’équipe déteste vraiment voir le nom de variable "myObkect".

Jay S
la source
1

Les réponses les plus votées ici sont très bonnes; celui-ci adresse l'angle de refactoring.

Dans la plupart des cas, la plupart des travaux de refactorisation consistent à comprendre le code existant; le changer après c'est généralement la plus petite partie du travail pour l'une des deux raisons suivantes:

  1. Si vous ne faites que rendre le code plus clair et / ou concis, les modifications nécessaires sont évidentes. Vous avez souvent acquis une compréhension du code en essayant des modifications qui semblaient plus simples et en voyant si elles fonctionnaient réellement ou si elles manquaient de subtilité dans le code plus complexe.

  2. Vous avez déjà à l’esprit une conception ou une structure particulière pour faciliter la création d’une nouvelle fonctionnalité. Dans ce cas, le travail de développement de ce design faisait partie de l'histoire qui en engendrait le besoin; il est indépendant de votre besoin de refactoriser pour arriver à cette conception.

Apprendre et comprendre le code existant représente une bonne quantité de travail pour un bénéfice non permanent (dans un mois, une personne aura probablement oublié beaucoup de choses sur le code si elle ne continue pas à lire ou à travailler avec ce code pendant tout ce temps), et ainsi de suite. cela n’a aucun sens, sauf pour les zones de code qui vous posent problème ou que vous prévoyez de modifier dans un proche avenir. À son tour, puisqu'il s'agit du travail principal du refactoring, vous ne devriez pas le refactoriser sur du code, sauf si cela vous cause actuellement des problèmes ou si vous prévoyez de le modifier dans un proche avenir.

Il existe toutefois une exception à cette règle: si une personne comprend bien le code et qu’elle perdra avec le temps, il peut s’avérer rentable de le clarifier et de le comprendre plus rapidement par la suite. C'est la situation dans laquelle se trouve quelqu'un qui vient de terminer l'élaboration d'une histoire.

Le refactor est un petit travail qui devrait être fait lors du prochain sprint (ou même avant qu'il ne commence) sous la forme d'une histoire minuscule à demi-point.

Dans ce cas, vous envisagez de créer une histoire distincte pour la refactorisation est un signal d'alarme sur plusieurs fronts:

  1. Vous ne pensez pas à la refactorisation dans le cadre du codage, mais plutôt comme une opération distincte, ce qui la rend susceptible d'être abandonnée sous la pression.

  2. Vous développez du code qui demandera plus de travail pour comprendre la prochaine fois que quelqu'un aura besoin de travailler dessus, ce qui rendra les récits plus longs.

  3. Vous perdez peut-être votre temps et votre énergie à refactoriser des choses pour lesquelles vous n’obtenez pas beaucoup d’avantages. (Si un changement se produit beaucoup plus tard, il faudra toujours que le code soit refait, alors de toute façon; cela est combiné plus efficacement avec le travail de refactoring. Si un changement ne se produit pas plus tard, le refactoring n'a servi à rien. but, sauf peut-être esthétique.)

Donc, la solution ici est d’échouer l’élément pour indiquer clairement que quelque chose de votre processus a échoué (dans ce cas, c’est le développeur ou l’équipe qui ne consacre pas le temps nécessaire à la révision et à l’implémentation des modifications apportées) et à la poursuite immédiate du travail par le développeur. sur l'article.

Lorsque vous allez faire une estimation pour la prochaine itération, ré-estimez l’histoire existante, car il ne vous reste plus qu’une charge de travail suffisante pour la faire passer en revue et l’ajouter à votre prochaine itération, tout en préservant l’estimation de l’itération précédente. Lorsque le récit est terminé à la fin de la prochaine itération, définissez la quantité de travail totale historique sur la somme des première et deuxième estimations afin de savoir combien de travail estimé a réellement été effectué. Cela aidera à produire des estimations plus précises d'histoires similaires à l'avenir, dans l'état actuel de votre processus. (Par exemple, ne supposez pas que votre apparente sous-estimation ne se reproduira plus; supposez que cela se reproduira jusqu'à ce que vous ayez terminé avec succès des histoires similaires tout en mettant moins de travail.)

Curt J. Sampson
la source
1

Je suis surpris par le manque de réponse dans les réponses et les commentaires à la notion d '"échec" d'une révision de code, parce que ce n'est pas un concept que je connais personnellement. Je ne serais pas non plus à l’aise avec ce concept ou avec les membres de mon équipe utilisant cette terminologie.

Votre question appelle explicitement les "pratiques agiles", alors revenons au manifeste agile (c'est moi qui souligne):

Nous découvrons de meilleures façons de développer un logiciel en le faisant et en aidant les autres à le faire. Grâce à ce travail, nous avons pu valoriser:

  • Individus et interactions sur les processus et les outils
  • Logiciel de travail sur une documentation complète
  • Collaboration client sur négociation de contrat
  • Répondre au changement suite à un plan

Autrement dit, même s'il y a de la valeur dans les éléments de droite, nous valorisons davantage les éléments de gauche.

Parlez à votre équipe. Discutez du code en question. Évaluez les coûts et les avantages et décidez, en tant que groupe d'experts cohérent, s'il est nécessaire de remodeler ce code maintenant, plus tard ou jamais.

Commencez à collaborer. Arrêtez les critiques de code qui échouent.

Ant P
la source
Je suis tout pour la collaboration. Mais quel terme utiliseriez-vous, sinon "échouer"? Même en discutant, en tant que groupe, une personne dirait "cela ne suffit pas, il faut un refactoring", ce qui signifie simplement que le contrôle de qualité a échoué, n'est-ce pas?
Erdrik Ironrose
1
@ErdrikIronrose Je n'ai jamais utilisé - ni eu besoin d'utiliser - la terminologie de "échec" d'une révision de code. Quelqu'un passe en revue le code, une discussion autour de tout point d'amélioration potentiel s'ensuit, suivie d'une décision sur l'opportunité de traiter ces points. Il n'y a pas de "réussite" ou "d'échec" impliqué, juste de la communication et du progrès. Je ne sais pas pourquoi on a besoin d'un tampon en caoutchouc.
Ant P
0

dans la critique, nous découvrons une fonction qui fonctionne, qui est lisible, mais qui est assez longue et qui a quelques odeurs de code ...

Existe-t-il des problèmes ou des considérations inhérents au fait d'élever un ticket à partir du dos d'un examen, au lieu de le rater?

Aucun problème que ce soit (de l'avis de mes équipes). Je suppose que le code répond aux critères d’acceptation énoncés dans le ticket (c’est-à-dire que cela fonctionne). Créez un élément de backlog pour traiter la longueur et toutes les odeurs de code et attribuez-lui une priorité, comme pour tout autre ticket. S'il est vraiment petit, donnez-lui une priorité élevée pour le prochain sprint.

L’un de nos dictons est le suivant: "Choisissez l’amélioration progressive plutôt que la perfection différée".

Nous avons un processus très fluide et construisons un assez bon nombre de fonctionnalités de "preuve de concept" (1 ou 2 par sprint) qui passent à travers les tests et les développements mais ne dépassent jamais l'examen interne des parties prenantes (hmm, pouvons-nous le faire à la place) ?), alpha ou beta ... certains survivent, d’autres pas.

Dans le projet actuel, je ne sais plus combien de fois nous avons créé une fonctionnalité donnée, l'avons mise entre les mains des parties prenantes et un ou deux sprints plus tard, l'avons totalement supprimée car l'orientation du produit a changé ou les exigences ont été créées. refonte complète de la manière dont la fonctionnalité doit être implémentée. Toute tâche d '«affinement» restante pour une fonctionnalité supprimée ou ne correspondant pas aux nouvelles exigences est supprimée ainsi qu'une partie de la gestion de l'arriéré.

chien de garde
la source
0

À mon avis, il y a deux façons d'aborder ce problème:

  1. La voie académique
  2. La manière du monde réel

D'un point de vue académique, la plupart des processus de révision de code permettent d'empêcher le déploiement d'un PBI (élément de backlog du produit) lorsque la norme de qualité du code n'est pas respectée.

Cependant, personne dans le monde réel ne suit agile au T, car pour une raison (parmi de nombreuses raisons), différentes industries ont des exigences différentes. Ainsi, corriger le code maintenant ou contracter une dette technique (vous créeriez probablement un nouveau PBI) devrait être décidé au cas par cas. Si cela risque de compromettre le sprint ou une publication ou d'introduire un montant de risque déraisonnable, les parties prenantes de l'entreprise doivent être associées à la décision.

RandomUs1r
la source
2
personne dans le monde réel ne suit agile au T - il ne sera plus "agile" si nous avons des règles trop strictes, non?
Paŭlo Ebermann
@ PaŭloEbermann J'ai eu une conversation amusante avec une entreprise avec laquelle j'ai interviewé une fois. Ils ont affirmé que leur processus n'était pas agile, car ce n'était pas un exemple classique d'agile. Même si tout ce qu'ils faisaient était dans un esprit agile. Je le leur ai fait remarquer mais je n'ai rencontré que (essentiellement) "Non, nous ne suivons pas à la lettre une procédure agile établie, même si nous empruntons énormément les concepts. Par conséquent, nous ne sommes pas agiles". C'était assez bizarre.
VLAZ
Comme d'autres critiques l'ont fait remarquer, dans ce cas, il est possible de tirer une leçon de l'incapacité du code à réussir véritablement la révision. Il me semble que les gens de ce projet ne comprennent vraiment pas bien a) qu'il faut laisser du temps pour la révision et la correction de chaque récit, et b) le refactoring nécessaire pour laisser le code propre derrière est un élément essentiel de la récit. Dans ce cas, la meilleure chose à faire est d'échouer dans l'histoire pour indiquer clairement que ces choses ne sont vraiment pas optionnelles.
Curt J. Sampson
@Curt, je comprends que le mien est peut-être une vue impopulaire du point de vue du développeur (je suis un développeur aussi), mais l'entreprise doit vraiment venir en premier, ils signent les chèques de paie et cela mérite un certain respect. En ce qui concerne le temps qui reste, je vais encore une fois contester votre compréhension du monde réel, et vous devez comprendre que ce n'est pas toujours possible et que beaucoup de sprints sont serrés, car les développeurs ont aussi besoin de choses à faire à la fin du sprint. Ce n'est pas comme parce que, dans le code SOLID, un département peut faire des siennes 1/10 jours toutes les deux semaines et ne rien faire. Cela pourrait être formidable à court terme, mais ce n'est pas viable longtemps.
RandomUs1r
@ RandomUs1r Je travaille aussi dans le monde réel, je prends des raccourcis tout le temps et je fais toujours passer les affaires en premier, alors je ne pense pas que je manque de compréhension ici. Mais la description de l'OP n'était pas "normalement nous obtenons toujours ce droit et ce n'était qu'un rots mineurs", sinon il n'aurait pas posé la question. Comme je l'ai expliqué dans ma réponse, cela ressemble à un problème de processus et vous corrigez le problème en pratiquant correctement le processus avant de vous détendre.
Curt J. Sampson
-2

Ni l'un ni l'autre . Si la révision du code échoue, la tâche n'est pas terminée. Mais vous ne pouvez pas rater les critiques de code sur avis personnel. Le code passe; passer à la tâche suivante.

Cela devrait être un appel facile, et le fait que ce ne soit pas le cas suggère que vous n’avez pas suffisamment de règles écrites pour la révision du code.

  1. "La fonction est assez longue". Notez: Les fonctions doivent être inférieures à X lignes (je ne suggère pas que les règles relatives à la longueur des fonctions soient une bonne chose).

  2. "Il y a des odeurs de code". Notez: les fonctions publiques doivent avoir des tests unitaires de fonctionnalité et de performance, l'utilisation de la CPU et de la mémoire doit être inférieure à x et y.

Si vous ne pouvez pas quantifier les règles permettant de passer une révision de code, vous obtiendrez ces cas de ce qui est fondamentalement «du code que vous n'aimez pas».

Si vous échouez «code que vous n'aimez pas»? Je dirais non. Vous commencerez naturellement à réussir / échouer selon des aspects non liés au code: aimez-vous la personne? Est-ce qu'ils discutent fortement pour leur cas ou juste faire ce qu'on leur dit? Est-ce qu'ils passent votre code quand ils vous examinent?

En outre, vous ajoutez une étape non quantifiable au processus d’estimation. J’estime la tâche en fonction de la façon dont je pense qu’elle devrait être programmée, mais à la fin, je dois changer le style de codage.

Combien de temps cela va-t-il ajouter? Le même relecteur effectuera-t-il la révision ultérieure du code et acceptera-t-il le premier relecteur ou trouvera-t-il des modifications supplémentaires? Que se passe-t-il si je ne suis pas d'accord avec le changement et si je ne le fais pas pendant que je recherche un deuxième avis ou plaide le cas?

Si vous voulez que les tâches soient effectuées rapidement, vous devez les rendre aussi spécifiques que possible. L'ajout d'un vague niveau de qualité n'aidera pas votre productivité.

Re: Il est impossible d'écrire les règles !!

Ce n'est pas si difficile. Vous voulez vraiment dire "je ne peux pas exprimer ce que je veux dire par" bon "code" . Une fois que vous avez reconnu cela, vous pouvez voir que le problème de ressources humaines est évident si vous commencez à dire que le travail de quelqu'un n'est pas à la hauteur, mais vous ne pouvez pas dire pourquoi.

Ecrivez les règles que vous pouvez et discutez avec la bière de ce qui rend le code «bon».

Ewan
la source
6
Non, vous oubliez que «disposer d'une norme parfaite et sans ambiguïté applicable de manière universelle» n'est pas une condition préalable réaliste à la révision de code. Il y aura toujours de nouveaux types de problèmes que vous n'aviez pas encore pris en compte et vous devez donc pouvoir prendre une décision en territoire inconnu. Bien sûr, vous devez ensuite documenter cette décision pour qu’elle ne soit plus un territoire non cartographié, mais votre réponse repose sur l’hypothèse que vous pouvez en quelque sorte garantir l’absence de territoire non cartographié si vous définissez seulement les règles parfaites avant de les passer en revue. Vous mettez la charrue avant les bœufs.
Flater
5
Les absolus comme "les fonctions doivent être inférieures à x lignes" ne sont pas non plus la solution .
Blrfl
2
D'accord avec Blrfl. Les fonctions (en général) ne doivent pas dépasser 20 lignes. Mais en faire une règle absolue est une erreur. Les circonstances spécifiques prévalent toujours sur les règles générales: si vous avez une bonne raison de créer plus de 20 lignes, alors faites-le.
Matt Messersmith
1
Vous ne devriez pas avoir besoin de règles pour le code écrit selon une spécification légale ... Vous pouvez simplement avoir des directives et le fait que vous êtes probablement tous des adultes qui essaient d'atteindre le même objectif final (code fonctionnel, lisible, pouvant être maintenu). Le fait d’avoir tous les membres de l’équipe véritablement investis dans l’équipe et prêts à travailler ensemble est au cœur de Scrum. Par conséquent, si vous ne l’avez pas, alors Scrum n’est peut-être pas pour votre équipe de toute façon.
user3067860
2
@ Ewan Bien sûr. Mon point de vue était juste que le PO a une directive de longueur de fonction , pas une règle. Chaque fois que le seuil est défini, il fournit des conseils pour aider les utilisateurs à identifier le code difficile à gérer, mais ce n'est jamais une règle absolue. Si (comme le dit le PO), il est en fait parfaitement lisible et que les critiques s'accordent pour dire qu'il est parfaitement lisible et qu'il n'y a aucun problème à le tester de manière appropriée, alors la fonction est par définition de la bonne taille. L’examen reçoit peut-être une note d’une ligne disant «Oui, c’est plus long que prévu, mais nous sommes d’accord, c’est bien», et le travail est fait. Le refactoring après ce point est un placage d’or.
Graham