Comment choisir le code à réviser?

14

Je fais partie d'une équipe de sept développeurs dans une petite société de logiciels et j'essaie d'introduire des revues régulières de code et de conception de groupe. Nous avons effectué quelques examens dans le passé, mais cela a été sporadique. Je voudrais en faire une chose plus régulière.

J'ai lu Code Complete et d'autres ressources similaires et ils parlent de la façon d'effectuer des révisions de code, mais je n'ai pas pu trouver de meilleures pratiques sur la façon de choisir quoi réviser. Nous avons une base de code vieille de plus de huit ans et couvrant une variété de langues, donc il y en a beaucoup qui pourraient être examinés.

Voici certains des facteurs auxquels je peux penser qui pourraient affecter le choix:

  • Langage: C, Java, SQL, PL / SQL
  • Âge du code: nouveau code vs ancien code
  • Utilisation du code: code fréquemment utilisé vs code (effectivement) mort / peu utilisé
  • Importance du code: code critique vs code non critique
  • Développeur: code développeur junior vs code développeur senior

Je comprends que ce n'est pas une question avec une réponse définitive absolue, mais toute orientation serait utile.

Quelques questions périphériques:

Burhan Ali
la source

Réponses:

12

En général, vous devez tout revoir . Si une nouvelle demande a 2 000 LOC, tous les 2 000 LOC doivent être examinés.

C'est pourquoi il n'y a pas de meilleure pratique sur la façon de choisir quoi réviser.

Si vous approchez d'une grande base de code existante, jamais examinée auparavant, c'est la même chose lorsque vous devez réécrire une grande base de code existante et choisir par où commencer. Cela dépend fortement:

  • sur la base de code (un seul code monolithique serait plus difficile à réécrire / réviser qu'un ensemble de composants séparés, etc.),

  • votre contexte (pouvez-vous arrêter tout ce sur quoi vous travaillez et passer trois mois (trois ans?) à travailler uniquement sur la réécriture / révision, ou vous devez le faire par petits intervalles, uniquement lorsque vous avez du temps libre)?

  • le type de révision que vous faites (avez-vous une liste de contrôle des éléments à réviser? Selon les éléments de la liste de contrôle, vous voudrez peut-être revoir certaines parties en premier).

Si j'étais toi, je:

  • suivez le principe 80% -20%, mentionné dans le premier commentaire de la deuxième question que vous avez liée.

  • prendre en compte que 100%, étant un idéal, n'en vaut peut-être pas la peine. C'est comme une couverture de code à 100% pour les tests unitaires, sauf qu'une telle couverture de code est généralement impossible ou extrêmement coûteuse.

  • commencez par les parties du code que vous utilisez le plus et qui sont les plus importantes. Si la base de code possède une bibliothèque qui authentifie et enregistre les nouveaux utilisateurs sur votre site Web d'entreprise, examinez-la d'abord, car vous voulez certainement trouver des failles de sécurité avant les pirates.

  • utiliser les mesures existantes pour déterminer ce qui est le plus important à examiner. Si une partie de la base de code n'a aucun test unitaire, tandis qu'une autre partie, tout aussi importante, a une couverture de code de 85%, commencez par examiner la première partie. Si une partie de la base de code a été écrite par un développeur qui était connu pour être inexpérimenté et pour introduire plus de bugs que n'importe lequel de ses collègues, commencez par revoir son code en premier.

Arseni Mourzenko
la source
Si vous faites le TDD correctement, une couverture à 100% n'est pas seulement facile, elle est également inévitable et s'avère en fait être une barre très basse.
Jonathan Hartley
4

Commencez par examiner toutes les modifications que vous apportez au code; cela empêchera le problème de s'aggraver. Commencez ensuite à examiner le code en fonction de la fréquence des changements; ce seront les «problèmes».

Vous voudrez trouver un moyen de suivre que vous avez examiné une section de code afin que vous puissiez analyser la couverture de l'examen de votre code par rapport à d'autres préoccupations.

Si vous pouvez déterminer quel code n'est pas couvert par vos tests, cela devient une priorité plus élevée pour la révision.

retracile
la source
3

Passez en revue toutes les nouvelles modifications qui ont été apportées avant leur mise en production. scripts d'installation, code source, modifications de la base de données, tout! Le point essentiel de la révision du code est d'empêcher le mauvais code de le mettre en production. Que ce soit un mauvais schéma organisationnel ou simplement un bug introduit parce que quelque chose a été oublié.

La refactorisation du code actuel sur lequel vous travaillez va de pair avec la révision du code. Par exemple, lorsque j'examine le code, s'il y avait du code en double dans une classe qui contenait un correctif de bogue, même si le développeur n'a pas modifié ce code dans leur correctif, je ne le transmettrais pas. Je voudrais les faire revenir en arrière et supprimer le code en double.

Si vous refactorisez sans relâche, la révision du code devient utile. Sinon, c'est une perte de temps.

Si vous intégrez le processus de révision du code comme étape de votre processus de développement, la base de code s'améliorera avec le temps. Mieux encore, vous ne devez pas permettre à vos développeurs de prendre en charge de nouvelles fonctionnalités ou des corrections de bugs tant que le backlog de révision de code n'est pas vide. Cela garantit que la révision du code est effectuée.

S'il existe des zones connues qui doivent être refactorisées, mais cela prendra beaucoup de temps (c'est-à-dire 1 semaine ou plus). Créez ensuite un élément de travail pour cette refactorisation par lui-même et ajoutez-le en tant qu'élément à travailler.

Charles Lambert
la source
1
Je ne suis pas d'accord - laissez le code passer en production et revoyez-le quand vous le pouvez. L'astuce consiste à faire confiance à vos développeurs et à supposer qu'ils ne font pas d'énormes erreurs. S'ils le font (nous le faisons tous), vous pouvez résoudre et refactoriser les problèmes après l'enregistrement. Essayer d'arrêter tout le code d'atteindre la production avant la révision signifie généralement qu'aucun code ne va à la production (d'après mon expérience). Bien sûr, si vous ne faites pas confiance à vos développeurs, n'hésitez pas à les verrouiller avec les ciseaux bien aiguisés dans l'armoire fixe.
gbjbaanb
"Passez en revue toutes les nouvelles modifications qui ont été apportées avant qu'elles ne soient mises en production". Je suis principalement d'accord avec cela. "Mieux encore, vous ne devez pas autoriser vos développeurs à prendre en charge de nouvelles fonctionnalités ou à corriger des bogues tant que le backlog de révision de code n'est pas vide." J'adorerais faire cela, mais étant donné la réalité des pressions commerciales, ce n'est malheureusement pas réaliste.
Burhan Ali
J'ai toujours confiance en mes développeurs. Les développeurs sont ceux qui font la révision du code. De plus, la mise en place d'un processus pour s'assurer que la révision du code ne se fait en aucun cas reflète un manque de confiance dans la capacité des développeurs. Les développeurs, les chefs de projet et les propriétaires d'entreprise devraient tous être plus soulagés qu'il y ait une chose de moins à s'inquiéter, à savoir: ne pas oublier de faire la révision du code.
Charles Lambert
@BurhanAli - C'est très réaliste. Nos clients sont des clients de premier plan (pensez Microsoft) et nos cycles de sortie sont très rapides. Cela devrait prendre environ 30 minutes, peut-être une heure à l'occasion, pour qu'un développeur examine une modification. Si cela prend plus de temps, vous ne divisez probablement pas votre travail en suffisamment petits morceaux, ce qui est un problème tout à fait différent.
Charles Lambert
2
@gbjbaanb Vous avez laissé du code non révisé entrer en production? Sensationnel. Il ne s'agit pas de ne pas faire confiance à vos développeurs (vous pouvez demander à l'un de vos développeurs d'examiner le code d'un autre), il s'agit de trouver des erreurs manifestement évidentes
Rob
2

Commencez par examiner tout le nouveau code et les modifications apportées au code existant.

Lors de l'examen des modifications du code existant, le développeur doit suivre la règle de boyscout. Laissez le code mieux qu'il ne l'a trouvé.

Cela ne signifie pas que vous devez corriger l'ensemble du fichier pour être parfait. Mais cela ne devrait pas ajouter au désordre, cela devrait le rendre un peu meilleur. Peut-être en déplaçant les modifications dans de nouvelles classes correctement structurées et en laissant le reste du fichier de code d'origine tel quel (après tout, son «fonctionnement»).

Une fois que vous avez commencé à améliorer le code en examinant tout le nouveau code et les modifications, en tant que développeurs, vous devez savoir quels domaines de l'application nécessitent le plus de changements. Ensuite, passez en revue ceux-ci, discutez comment ils peuvent être améliorés, petit à petit.

La révision du code écrit il y a 10 ans, pour le revoir, est inutile, le développeur aurait dû s'améliorer au cours de ces 10 années. Il est donc inutile de l'examiner juste pour apprendre ce que vous savez tous.

Le but des revues de code est d'améliorer et de corriger les erreurs que vous faites actuellement et de partager ces connaissances au sein de l'équipe.

CaffGeek
la source
+1 pour "Laissez le code mieux qu'il ne l'a trouvé." J'essaie toujours d'encourager cela. Quant à revoir le code vieux de 10 ans juste pour le plaisir, je suis d'accord avec ce que vous dites. Mais y a-t-il un avantage à le faire pour rendre l'équipe plus à l'aise avec l'idée de réviser? Il n'y a pas beaucoup de danger que les gens deviennent défensifs sur du code qu'ils n'ont pas écrit (ou sont si vieux qu'ils ont oublié de l'avoir écrit.)
Burhan Ali
1

Dans mon projet, nous incluons la révision de code comme un must-have dans la plupart des cas pour toute affectation / histoire utilisateur / bogue en cours de développement. Nous utilisons des processus Scrum / Agile et le ticket / histoire n'est pas déplacé vers construit (c'est-à-dire un backlog pour QA) jusqu'à ce que les tests unitaires soient écrits et que la révision du code soit terminée.

Nous utilisons à cet effet l' analyse Atlassian FishEye avec la revue de code Crucible intégrée à JIRA + SVN.

Lorsque le développeur enregistre le code pour une histoire spécifique, il crée une nouvelle révision de code dans FishEye, où il sélectionne les autres membres de l'équipe pour effectuer la révision.

Une fois la révision du code terminée (l'outil met en évidence les modifications soumises et permet de laisser les commentaires pour la ligne de code spécifique), le développeur corrige les problèmes mentionnés / implémente les améliorations suggérées et déplace le ticket dans la colonne Construit dans JIRA - cela signifie que l'histoire est prêt à être testé et qu'aucun autre changement de code n'est attendu pour cet élément de travail spécifique.

Cela garantit également que QA ne teste rien qui pourrait être modifié et potentiellement cassé lors de la refactorisation du code après la révision du code .

Pour résumer, tout le code doit être examiné - cela prend en charge la haute qualité du code, ce qui se traduit généralement par une meilleure conception, lisibilité, maintenabilité et testabilité du code et améliore les performances de développement à long terme.

Paul
la source