OK, donc beaucoup de révision de code est assez courante. Mais il arrive parfois que des changements aient un impact important sur le code complexe et fragile existant. Dans cette situation, le temps nécessaire pour vérifier la sécurité des modifications, l'absence de régression, etc. est excessif. Peut-être même dépasser le temps nécessaire pour réaliser le développement lui-même.
que-faire dans cette situation? Fusionner et espérer que rien ne passe? (Ne préconisez pas cela!) Ne faites-vous pas le mieux et essayez-vous de repérer les failles évidentes (c'est peut-être la révision de code la plus recherchée?) Fusionner et tester de manière plus approfondie comme meilleure alternative que la révision de code?
Ce n'est pas précisément une question de savoir si les tests doivent être effectués dans le cadre d'une révision de code. C'est une question qui demande quelles sont les meilleures options dans la situation telle que décrite, en particulier avec une échéance pressante, aucune suite complète de tests unitaires disponibles ou des tests unitaires non viables pour le code fragmenté modifié.
EDIT: J'ai l’impression que quelques réponses / commentaires jusqu’à présent ont repris ma phrase «impact général», et ont probablement interprété cela comme signifiant que le changement impliquait un grand nombre de lignes de code. Je peux comprendre que ce soit l'interprétation, mais ce n'était pas vraiment mon intention. Par "impact général", je veux dire, par exemple, le potentiel de régression est élevé en raison de l'interdépendance du code, ou de la portée des effets d'entraînement, sans que le changement lui-même soit important. Par exemple, un développeur peut trouver un moyen de résoudre un bogue avec une seule ligne en appelant une routine de haut niveau existante qui transfère des appels en cascade à de nombreuses routines de niveau inférieur. Il est facile de tester et de vérifier que le correctif de bogue a fonctionné. La validation manuelle (via la révision du code) de l’impact de tous les effets en chaîne est beaucoup plus difficile.
la source
what if there is no pre-existing test suite?
- Pourquoi ne pas en écrire un?Merge and hope nothing slips through?
C'est une mauvaise idée notoirement.Réponses:
La prémisse de la question est, franchement, ahurissante. Nous supposons qu'il y a un changement important dans le code fragile et complexe et qu'il ne reste tout simplement pas assez de temps pour le réviser correctement . C'est le dernier code sur lequel vous devriez passer moins de temps à réviser! Cette question indique que vous avez des problèmes structurels non seulement dans votre code, mais également dans votre méthodologie de gestion du changement.
Alors, comment faire face à cette situation? Commencez par ne pas y entrer en premier lieu:
Identifiez les sources de complexité et appliquez des refactorings corrects, soigneusement révisés et corrects pour augmenter le niveau d'abstraction. Le code doit être compréhensible pour un nouvel employé récemment diplômé qui connaît quelque chose du domaine de votre entreprise.
Identifier les sources de fragilité; Cela peut être effectué en examinant le code lui-même, en examinant l'historique des corrections de bogues apportées au code, etc. Déterminez quels sous-systèmes sont fragiles et rendez-les plus robustes . Ajouter une logique de débogage. Ajouter des assertions. Créez une implémentation lente mais évidemment correcte du même algorithme et, dans votre version de débogage, exécutez les deux et vérifiez qu'ils concordent. Dans votre version de débogage, les situations rares se produisent plus fréquemment. (Par exemple, créez un allocateur de mémoire qui déplace toujours un bloc lors de la réallocation ou alloue toujours un bloc à la fin d'une page, ou quoi que ce soit.) Rendez le code robuste face aux modifications apportées à son contexte. Maintenant, vous n'avez plus de code fragile; Maintenant, vous avez un code qui trouve les bugs, plutôt que provoque les bugs.
Écrire une suite de tests automatisés. Évidemment.
Ne faites pas de grands changements. Faites une série de petits changements ciblés, chacun pouvant être considéré comme correct.
Mais fondamentalement, votre scénario est le suivant: "nous nous sommes plongés dans un gouffre de dette technique et chaque changement complexe non examiné nous a creusés plus profondément; que devrions-nous faire?". Que faites-vous lorsque vous vous trouvez dans ce trou? Arrête de creuser . Si vous avez tellement de dettes que vous ne pouvez pas effectuer de tâches de base, telles que la révision du code de chacun, vous devez cesser de vous endetter davantage et consacrer du temps à les rembourser.
la source
L'un des principaux objectifs d'une révision de code est d' améliorer la qualité et de fournir un code robuste . Robuste, parce que 4 yeux repèrent généralement plus de problèmes que 2. Et le relecteur qui n'a pas écrit le code supplémentaire est plus susceptible de remettre en cause les hypothèses (potentiellement fausses).
Éviter les examens par les pairs ne contribuerait dans votre cas qu'à accroître la fragilité de votre code. Bien entendu, le renforcement des tests avec une suite de tests solide et répétable pourrait certainement améliorer la qualité. Mais cela devrait être complémentaire à l'examen par les pairs, pas un remplaçant .
Je pense que la complexité doit être comprise et maîtrisée, et l’ évaluation complète par les pairs est l’occasion de partager des connaissances et d’y parvenir. L'investissement que vous faites pour que davantage de personnes comprennent la force et la faiblesse de ce code fragile contribuera à l'améliorer au fil du temps.
Une citation pour conclure:
la source
Bienvenue dans le monde du développement de logiciels existants.
Vous avez des centaines de milliers, des millions, des dizaines de millions de lignes de code.
Ces lignes de code ont de la valeur, en ce sens qu’elles génèrent un flux de revenus et qu’il est impossible de les remplacer.
Votre modèle commercial repose sur l'exploitation de cette base de code. Donc, votre équipe est petite, la base de code est grande. L'ajout de fonctionnalités à est nécessaire pour amener les utilisateurs à acheter une nouvelle version de votre code ou pour satisfaire les clients existants.
Dans un monde parfait, votre énorme base de code est testée d'un seul coup. Vous ne vivez pas dans un monde parfait.
Dans un monde moins parfait, vous avez le budget pour réparer votre dette technique: décomposez votre code en éléments testables par unité, effectuez des tests d'intégration approfondis et effectuez une itération.
Ceci, cependant, réduit la dette sans produire de nouvelles fonctionnalités. Ce qui ne correspond pas à l'analyse de rentabilisation de "tirer des bénéfices du code existant, tout en le modifiant afin de générer une incitation à la mise à niveau".
Vous pouvez prendre d'énormes morceaux de code et le réécrire en utilisant des techniques plus modernes. Mais partout où vous interagissez avec le code existant, vous exposez des points de rupture possibles. Ce hack dans le système dont vous vous êtes débarrassé vous a en fait indemnisé pour un caprice dans un sous-système que vous n'avez pas réécrit. Toujours.
Ce que vous pouvez faire, c'est agir avec prudence. Vous pouvez trouver une partie du code que vous comprenez réellement et dont le comportement et l'interaction avec le reste du système sont bien compris. Vous pouvez le moderniser en ajoutant des tests unitaires et en rendant son comportement encore plus clair.
Trouvez ensuite les parties du reste de l'application qui interagissent principalement avec elle et attaquez-les un à un.
En procédant ainsi, vous pouvez améliorer le sous-système en ajoutant des fonctionnalités que les clients sont prêts à payer.
En bref, il s’agit de l’art du possible - apporter des modifications sans casser des choses qui fournissent une analyse de rentabilisation.
Mais ce n'est pas ta question. Votre question est la suivante: "Je fais quelque chose qui est énorme et susceptible de casser des choses, et comment puis-je suivre les meilleures pratiques?"
Lorsque vous faites quelque chose d'énorme, il est vrai que si vous voulez le faire de manière fiable, vous finissez par consacrer plus d'effort à la recherche des bogues et à leur réparation que de l'écrire. Telle est la règle générale du développement logiciel: écrire des choses est facile, il est difficile de les faire fonctionner parfaitement.
Vous avez probablement une analyse de rentabilisation au-dessus de votre tête, dans laquelle vous avez promis à un intervenant que ce changement massif intervient. Et c'est "fait", vous êtes donc repoussé en disant "non, ce n'est pas fait, c'est juste J'aime ça".
Si vous avez le pouvoir et le budget, consacrez réellement vos efforts à générer la confiance que le changement fonctionne, ou refusez simplement le changement. Cela va être une question de degré, pas gentil.
Si vous n'avez pas beaucoup de puissance, mais en avez quand même, essayez d'insister pour que le nouveau système soit testable par unité . Si vous réécrivez un sous-système, insistez sur le fait que le nouveau sous-système est composé de petites pièces dont le comportement et les tests unitaires sont bien spécifiés.
Ensuite, il y a le pire des cas. Vous allez plus loin dans la dette. Vous empruntez contre l'avenir du programme en disposant de plus de code fragile et de plus de bugs afin de pouvoir publier la fonctionnalité maintenant , et bien des conséquences. Vous effectuez un contrôle qualité basé sur le balayage pour identifier les problèmes les plus graves et ignorer les autres. C’est en fait parfois la bonne réponse du point de vue de l’entreprise, comme c’est le moins cher actuellement. S'endetter pour générer des profits est une stratégie commerciale valable, en particulier si le règlement de la dette par la faillite (abandon du code) est à l'ordre du jour.
Un gros problème tient au fait que les incitations des propriétaires d’entreprise sont rarement alignées sur les décideurs et les programmeurs. Il y a généralement beaucoup de pression pour «livrer», et générer une dette technique presque invisible (pour vos supérieurs) est une excellente stratégie à court et parfois à moyen terme. Même si vos supérieurs / parties prenantes seraient mieux servis en ne créant pas toute cette dette.
la source
Résolvez les problèmes plus importants qui rendent la révision de code trop difficile.
Ceux que j'ai repérés jusqu'à présent:
la source
Vous pouvez renvoyer la révision de code et demander au développeur de la diviser en ensembles de modifications plus petits et plus incrémentaux et de soumettre une révision de code plus petite.
Vous pouvez toujours vérifier les odeurs de code, les modèles et les anti-modèles, les normes de formatage du code, les principes SOLID, etc. sans nécessairement passer par tous les détails du code.
Vous pouvez toujours effectuer des inspections de code tactique pour une validation d'entrée correcte, une gestion des verrous / threads, des exceptions possibles non gérées, etc. à un niveau détaillé, sans nécessairement comprendre l'intention globale de l'ensemble de modifications.
Vous pouvez fournir une évaluation des zones de risque globales que le code peut affecter, et demander au développeur de confirmer que ces zones de risque ont été testées à l'unité (ou de lui demander d'écrire des tests unitaires automatisés, et de les soumettre également à l'examen). ).
la source
Les revues de code ne doivent pas être principalement axées sur l'exactitude. Ils sont là pour améliorer la lisibilité du code, la maintenabilité et le respect des normes de l'équipe.
La recherche de bogues de correction lors de la révision d'un code est un sous-produit intéressant, mais un développeur doit s'assurer que son code fonctionne parfaitement (y compris la non régression) avant de le soumettre pour révision .
La justesse devrait être intégrée dès le départ. Si un développeur ne peut pas l'obtenir, demandez-lui de jumeler un programme ou de définir un plan avec toute l'équipe, mais ne le considérez pas comme quelque chose que vous pouvez ajouter après coup.
la source
Si vous pensez que l'examen du code est trop difficile, car il a modifié un code fragile qu'il est presque impossible de modifier sans le casser, vous avez un problème. Mais le problème ne vient pas de la révision du code. Le problème ne réside pas non plus dans les tests unitaires, car le code fragile ne peut pas être testé à l'unité! Si votre code était testable par unité, il aurait été divisé en petites unités indépendantes, chacune pouvant être testée et fonctionnant bien ensemble, et c'est exactement ce que vous n'avez pas!
Donc, vous avez un tas de code de déchets (aka "dette technique"). La pire chose à faire est de commencer à réparer ce tas de code de déchets et de ne pas terminer le travail, car vous obtiendrez un tas encore plus important de code de déchets. La première chose à faire est donc de faire en sorte que votre direction accepte de le réparer et de terminer le travail. Ou tu ne le fais pas. Dans ce cas, vous n'y touchez pas.
Lorsque vous corrigez le problème, vous extrayez une unité du code, vous le transformez en un comportement bien défini et bien documenté, vous écrivez des tests unitaires pour cette unité, vous le passez en revue par le code et vous priez pour que rien ne se casse. Et puis vous faites la même chose avec l'unité suivante, et ainsi de suite.
La difficulté vient quand vous rencontrez des bugs. Votre nid de code de rats fera de mauvaises choses dans certains cas parce que les choses sont si fragiles et compliquées que les choses vont mal se passer. Lorsque vous extrayez des unités, le code restant deviendra plus clair. (Dans certains cas, après une refactorisation, une fonction a été lancée avec "if (condition1 && condition2 && condition3) crash ();", ce qui était exactement le comportement précédent la refactorisation, mais plus clair. J'ai ensuite supprimé cette ligne :-) Vous verrez comportement étrange et indésirable clairement, de sorte que vous pouvez y remédier. D'autre part, c'est là que vous devez changer le comportement du code existant, il faut donc le faire avec précaution).
la source
Malheureusement, vous ne pouvez rien faire à ce sujet au moment de la révision du code, à part prendre une autre tasse de café. La solution réelle à ce problème consiste à remédier à la dette technique que vous avez accumulée: conception fragile, manque de tests. J'espère que vous avez au moins une sorte d'AQ fonctionnelle. Si vous n'en avez pas, il y a toujours des prières pour des os de poulet.
la source
Si vous n'êtes pas content d'expédier avec un logiciel défectueux / non fonctionnel et de le réparer plus tard, l'effort de V & V DEVRAIT être plus long que l'effort de développement!
Si le code existant est fragile, la première question est "devriez-vous même le changer?" La direction doit déterminer si le coût / risque de la refonte et de la réimplémentation de ce code est supérieur au coût / risque de la correction de la pile de déchets. Si c'est un cas isolé, il peut être plus facile de le corriger. S'il est probable que davantage de changements seront nécessaires à l'avenir, prendre la décision immédiatement pour éviter davantage de souffrances à l'avenir pourrait être une meilleure décision. Vous devez en parler avec votre direction, car donner à vos gestionnaires de bonnes informations fait partie de votre travail. Ils doivent prendre cette décision, car c’est une décision stratégique qui dépasse votre niveau de responsabilité.
la source
D'après mon expérience, je vous recommande fortement de couvrir votre code avec un nombre assez important de tests, à la fois d'unité et d'intégration, AVANT que des modifications ne soient apportées au système en question. Il est important de se rappeler qu'aujourd'hui, il existe un très bon nombre d'outils à cette fin, peu importe le langage que vous développez.
En outre, il existe L’UN des outils pour vous permettre de créer vos tests d’intégration. Oui, je parle de conteneurs et spécialement de Docker et Docker Compose . Il nous fournit magnifiquement un moyen de configurer rapidement un environnement d’application complexe, avec une infrastructure (base de données, mongodb, serveurs de files d’attente, etc.) et des applications.
Les outils sont disponibles, utilisez-les! :)
la source
Je ne sais pas pourquoi cela n'a pas encore été mentionné, mais ces 2 sont les pièces les plus importantes:
* Exemple: vous remplacez la bibliothèque A par la bibliothèque B. Une liste de modifications présente la bibliothèque B, plusieurs listes de modifications différentes remplacent l'utilisation de A par B pièce par pièce (par exemple, une liste de modifications par module) et la dernière liste de modifications supprime la bibliothèque A.
la source
Ne sous-estimez pas la valeur potentielle des revues de code. Ils peuvent être bons pour détecter les bugs:
Ils sont également utiles pour d'autres raisons:
Dans le meilleur des cas / l'idéal, passer l'inspection du code ne signifie pas simplement "pas de bugs évidents": cela signifie "évidemment pas de bugs" (bien que vous souhaitiez bien sûr le tester également).
Si vous ne pouvez pas vérifier la nouvelle base de code via l'inspection du code, des tests plus approfondis de type "boîte noire" seront alors nécessaires. Vous êtes peut-être habitué à un cycle de développement dans lequel vous mettez du code en production après le passage de l'inspection, mais s'il ne réussit pas à "inspecter", vous ne pouvez pas le "mettre en production" et nécessite un cycle plus long: tests d'intégration, par exemple. , tests système, tests alpha, tests de réception, tests bêta, etc.
Qu'en est-il des tests d'intégration, de système et d'acceptation?
Quoi qu'il en soit, vous devriez probablement dire au chef de projet et au chef de produit que le code est presque certainement douteux, avec un nombre inconnu de bogues; et qu'ils "obtiendront ce qu'ils inspecteront" au lieu d'obtenir simplement "ce qu'ils attendent" - c'est-à-dire que la qualité du code ne sera pas meilleure que leurs tests (car la qualité du code n'a pas été et ne peut pas être garantie par l'inspection du code) .
Ils devraient éventuellement relayer ce message au client ou aux utilisateurs, de sorte qu'ils effectuent des tests bêta (s'ils souhaitent adopter précocement), ou utilisent l'ancienne version jusqu'à ce que la nouvelle version ne soit plus en version bêta (si ce n'est pas le cas).
la source
Beaucoup de code est écrit et fusionné sans révision appropriée du code. Ça peut marcher. Il y a une raison pour laquelle cela s'appelle code odeur pas "code cassé" ou quelque chose à cet effet. L'absence de révision du code est un signe avant-coureur et non un présage de malheur.
La solution à ce problème est qu’il n’existe pas de solution unique pour tous les cas pouvant être intégrés dans une réponse de style StackExchange. La communauté du développement de logiciels est fermement convaincue que la révision du code est une "meilleure pratique" cruciale et, dans ce cas, elle est ignorée. Votre développement ne fait plus partie de ce canal étroit consistant à "suivre toutes les meilleures pratiques". Vous devrez trouver votre propre chemin.
Qu'est-ce qu'une "meilleure pratique" de toute façon? En fin de compte, il s'agit d'un ensemble de pratiques qui, de l'avis général, améliorent le code. Est-ce qu'ils font le code juste? Zut non! Internet est jonché d’histoires d’entreprises qui ont suivi les «meilleures pratiques» et se sont laissées emporter par elle. Peut-être un meilleur point de vue sur les «meilleures pratiques» est-il que ce sont les solutions «feu et oublie» du monde des logiciels. Je ne peux rien savoir de votre entreprise, de votre projet, de votre équipe, et être en mesure de présenter les "meilleures pratiques" comme des choses qui vous aideront. Ils sont le conseil général "ne pas nuire".
Vous avez clairement dévié de ce plan. Heureusement, vous le reconnaissez. Bon travail! Ils disent que la connaissance est la moitié de la bataille; si c'est le cas, la conscience en dépasse largement la moitié! Maintenant, une solution est nécessaire. D'après votre description, il est clair que l'environnement commercial dans lequel vous vous trouvez a évolué à un point tel que le conseil ennuyeux consistant à "passer en revue le code, il est de la meilleure pratique" ne va pas le couper. Pour cela, je recommande une règle clé que j'utilise lorsque je parle des meilleures pratiques logicielles:
Franchement, ils paient votre salaire et la survie de l'entreprise est généralement bien plus importante que la qualité du logiciel. Nous n'aimons pas l'admettre, mais un logiciel parfaitement écrit est inutile s'il est piégé dans le corps d'une entreprise en train de mourir de ses efforts pour maintenir ce logiciel parfaitement écrit.
Alors où vas-tu? Suivez le sentier de la force. Vous avez souligné que, pour une raison non spécifiée, il est déraisonnable de procéder à une révision du code pour une tâche quelconque. D'après mon expérience, cette raison est toujours temporelle. C'est toujours soit "pas assez de temps" ou "pas assez d'argent pour que les salaires continuent de couler pendant que vous passez votre temps". C'est des affaires; ça va. Si c'était facile, tout le monde le ferait. Suivez la trace de la force vers le haut et trouvez la direction en mesure de vous aider à comprendre pourquoi une révision du code n'est pas une option. La langue est difficile et, très souvent, un décret tombe des mains de la haute direction et se déforme. La solution à votre problème peut être cachée dans cette distorsion.
La réponse à cette question est nécessairement un scénario de cas particulier. C'est comme essayer de prédire si un tirage au sort se fera en tête ou en queue. Les meilleures pratiques disent de le retourner 100 fois et l’attente sera d’environ 50 têtes et 50 queues, mais vous n’avez pas le temps de le retourner une fois. C'est ici que les détails de votre situation importent. Saviez-vous qu'une pièce atterrit généralement dans la même orientation que 51% du temps au moins? Avez-vous pris le temps de regarder de quel côté était la pièce avant de lancer? Cela pourrait faire une différence.
Une solution générale à votre disposition consiste à essayer de trouver un moyen de définir le processus de révision du code et d’en faire un effort très économique. Une grande partie du coût d'un processus de révision de code est que tout le monde se consacre à 100% à la révision de code pendant que vous le faites. Cela doit être le cas car, une fois que la révision du code est terminée, le code est béni. Vous pouvez peut-être placer le code dans une autre branche et faire la révision du code en parallèle avec le développement sur le tronc principal. Ou peut-être pouvez-vous même le configurer pour que le logiciel effectue les tests pour vous. Vous vous trouvez peut-être dans un environnement professionnel dans lequel vos clients peuvent exécuter le "nouveau" code en parallèle avec l'ancien et leur demander de comparer les résultats. Cela transforme les clients en un tas de dispositifs de création de cas d'utilisation.
Une des clés de toutes ces opérations "maybes" est que vous devez vous efforcer de diviser facilement votre code. Vous serez peut-être en mesure de "valider" certaines parties du code sans recourir à une révision formelle du code en les utilisant dans des projets moins critiques. Il est plus facile de le faire si les modifications sont réduites en petites quantités, même si leur somme totale est trop importante pour être examinée par les pairs.
De manière générale, recherchez des solutions spécifiques à votre projet, votre entreprise, votre équipe. La réponse générale était "les meilleures pratiques". Vous ne les utilisez pas, vous devriez donc chercher des solutions plus personnalisées à ce problème, cette fois-ci. C'est des affaires. Si tout se passait comme prévu, les IPO seraient beaucoup plus faciles à attribuer à des valeurs, n'est-ce pas!
Si le remplacement d'une révision de code est une tâche ardue, rappelez-vous qu'il n'y a jamais eu un seul élément de code qui ait fait ses preuves lors d'une révision de code. avant qu'ils ne deviennent un problème. Ces deux produits de valeur d’une révision de code peuvent être acquis par d’autres moyens. La révision de code a juste une valeur reconnue pour être particulièrement douée.
* Et bien, presque: le micro-noyau L4 a été révisé il y a quelque temps par un système de preuve automatique qui prouve que son code, s'il est compilé par un compilateur C ++ conforme, fera exactement ce que dit la documentation.
la source
Comme @EricLippert le souligne dans son excellente réponse, ce type de changement nécessite plus d' attention, pas moins . Si vous réalisez qu'un changement sur lequel vous travaillez va devenir un tel changement, voici quelques stratégies qui pourraient vous aider:
la source
Plus de réponses abordent comment vous en êtes arrivé à ce point. Beaucoup d’entre eux font quelques suggestions pour remédier à la situation, mais je voudrais ajouter ma réponse pour donner une réponse courte.
Que faire lorsque les critiques de code sont "trop difficiles?"
Oui
Vos développeurs étaient géniaux! Les chats reviennent pour tout le monde!
(ou pour ceux qui n'ont pas grandi en regardant " The Simpsons " à la télévision américaine: si les tests sont réussis, évitez d'essayer de regarder les différences et de laisser le développeur vous guider vers les modifications)
Non
Continuez à refactoriser et à ajouter une couverture de test jusqu'à la réussite des tests.
la source
Comme une multiplication, la révision de code donne un résultat nul lorsqu'elle est appliquée à zéro. Cela n'augmente pas la valeur dans un tel cas, alors que dans la plupart des autres cas, ce serait le cas.
Le code sur lequel vous devez travailler est trop mal conçu pour tirer parti du processus de révision du code au cours du développement ultérieur. Utilisez le processus de révision du code pour le refactoriser ou le re-développer.
Il se peut aussi que le code soit toujours supportable mais la tâche n’est pas bonne. Il est trop large et aurait dû être fait par petites étapes.
la source