Aujourd'hui, de nombreuses personnes utilisent des revues de code dans le développement. La pratique est utile, nécessaire. Même si vous ne faites pas d'examen, vous savez probablement ce que c'est.

Il existe de nombreux outils de révision de code sur le marché avec des scénarios d'utilisation, des recommandations et des règles prêts à l'emploi. GitHub, Phabricator, FishEye / Crucible, GitLab, Bitbucket, Upsource - la liste s'allonge encore et encore. Chez Badoo, nous avons également travaillé avec eux à un moment donné: dans mon article précédent, j'ai raconté notre histoire sur les revues de code et comment nous avons inventé notre propre «vélo» - les solutions Codeisok .

Il y a beaucoup d'informations, vous pouvez rechercher sur Google un tas d'articles sur les revues de code avec des exemples réels, des pratiques, des approches qui parlent de ce qui est bon, mauvais, comment faire et comment - vous n'avez pas besoin de, ce que vous devez considérer et ce qui ne l'est pas, etc. e. En général, le thème est «aspiré jusqu'aux os».

C'est pourquoi l'autre partie de l'iceberg peut ne pas être remarquée.

J'espère que vous prendrez au sérieux les choses que je décrirai dans cet article, dans certains cas exagérant délibérément. Un examen, comme tout autre processus, nécessite une mise en œuvre minutieuse et délibérée, tandis que l'approche «Nous ferons comme tout le monde, si seulement» peut non seulement échouer, mais même nuire.

Après avoir lu cette introduction, vous pouvez avoir une question: qu'est-ce qui est compliqué à propos de la révision du code? Le point est insignifiant. De plus, la plupart des outils énumérés ci-dessus offrent immédiatement un examen du flux (prêt à l'emploi: définir, valider / démarrer - et c'est parti!).

Mais la pratique montre que tout n'est pas aussi évident qu'il y paraît à première vue. Y compris en raison de la simplicité imaginaire du processus. Lorsque tout est opérationnel, il y a un risque que le gestionnaire se calme et cesse de s'enfoncer dans le processus, le transmettant aux développeurs. Et ils suivront le processus ou feront autre chose au détriment de la résolution des problèmes que le processus d'examen est censé résoudre. Le gestionnaire peut ne pas être conscient que quelque chose ne va pas, car il ne fixe pas les règles ou n'impose pas de règles. Bien qu'il soit très important pour une entreprise de résoudre les problèmes le plus rapidement possible, sans perdre de temps sur des activités imprévues .

Il était une fois, lorsque je travaillais dans une autre entreprise, j'étais profondément submergé par la conversation sur les revues de code avec l'un des principaux développeurs. C'était p1lot . Peut-être qu'un des lecteurs le connaît (p1lot, bonjour :)). Il travaille actuellement avec nous à Badoo, ce qui est formidable.

Ainsi, PILOT a ensuite déclaré: "La chose la plus difficile pour moi dans le processus de révision est de compromettre et d'ignorer davantage la tâche terminée et fonctionnant correctement, même si je connais une autre façon de la résoudre et même si j'aime davantage ma méthode." À mon avis, c'est une pensée clé. Et le message principal de tout mon article tourne autour de ce postulat.

Pourquoi ai-je besoin d'un processus de révision de code?

Avez-vous un avis dans votre entreprise? Le faites-vous bien? J'ai un test qui peut vous en faire douter.

Vous devez répondre à seulement trois questions:

1. Combien de temps prend une revue de code pour une tâche moyenne (sphérique dans le vide)?
   
2. Comment minimisez-vous le temps d'examen?
   
3. Comment déterminez-vous si l'examen d'une tâche spécifique est effectué correctement?
   

Si vous n'avez pas de réponses claires à certaines (ou à toutes) les questions, si vous doutez de vos réponses, alors j'ose supposer que quelque chose ne va pas.

Si vous ne savez pas combien de temps il faudra pour passer en revue et que vous ne le réduisez pas constamment, comment planifiez-vous? Est-il possible que l'interprète, qui a fait la revue pendant quatre heures, n'ait pas bu du thé à moitié cette fois? Est-il possible d'être sûr que le temps pour boire du thé n'augmente pas d'une tâche à l'autre? Ou peut-être que le réviseur ne regarde pas du tout le code, mais clique simplement sur «Le code est OK»?

Et, même si l'examen est fait de bonne foi, comment pouvons-nous nous assurer qu'avec la croissance de la base de code et de l'entreprise, le temps nécessaire pour terminer les tâches n'augmente pas? Après tout, la direction s'attend, en règle générale, à ce que les processus accélèrent et non ralentissent.

Si la réponse à la troisième question ne vous vient pas immédiatement à l'esprit, il est logique d'en poser une de plus. Savez-vous pourquoi vous avez vraiment besoin de ce processus? Parce que "est si habituel pour tous les grands"? Peut-être que vous n'avez pas du tout besoin de lui?

Si vous demandez à vos prospects et développeurs ce qu'est la révision de code «correcte», vous serez très surpris de la façon dont les différentes choses que vous entendez. Les réponses peuvent varier en fonction de l'expérience personnelle, des croyances, de la confiance personnelle dans l'exactitude ou l'inexactitude des choses, et même en fonction de la pile technologique utilisée et du langage de développement.

Vous vous souvenez des outils prêts à l'emploi pour la révision de code, offrant leurs propres approches et flux? Par exemple, je me demande comment les développeurs de ces outils répondraient à la question. Leurs réponses sur la «justesse» de l'examen seront-elles en corrélation avec les réponses de vos employés? Pas sûr. Parfois, je regarde tristement comment quelqu'un met en œuvre des outils de révision à la maison, ne doutant pas une minute qu'ils sont nécessaires. Soit les gens le font «pour le spectacle», soit pour signaler qu '«ils ont mis en œuvre la révision du code, tout va bien». Et à la fin, ils l'oublient.


Je ne veux pas être Cap, mais néanmoins. Lorsque vous communiquez avec les employés, faites attention aux réponses comme «Parce que c'est tellement établi» ou «C'est la meilleure pratique, tout le monde le fait». Vous savez vous-même que vous n'avez pas besoin de mettre en œuvre un processus sans réfléchir sans comprendre pourquoi il est nécessaire. Tout processus doit être justifié et mis en œuvre (si nécessaire) ajusté aux besoins de l'entreprise et du projet, ainsi qu'aux problèmes qui existent vraiment et que vous voulez vraiment résoudre. Le culte du fret dans une entreprise avec une bonne culture d'ingénierie n'appartient pas.

En conséquence, il est important que le manager transmette aux gens la révision de code «correcte» qui est nécessaire précisément dans son projet. Et avant cela, bien sûr, les critères de «correction» doivent être formulés pour vous-même, choisir les bons outils et établir les règles appropriées. Et il y a déjà près de contrôler.

Mauvaise revue de code

Alors, quel est le bon processus? Faisons les choses correctement. Ci-dessous, il y aura beaucoup de mon raisonnement personnel, et pour ceux qui ne peuvent pas attendre pour savoir ce que j'écris tout cela, je propose de passer immédiatement à la partie "Bonne révision du code".

À ceux qui sont restés, je dis merci et néanmoins je propose de déterminer par moi-même l'exactitude de l'examen, en fonction des besoins de votre projet, en prenant soigneusement tout en considération. Et j'essaie juste de vous aider avec ça.

Afin de mettre en évidence les choses importantes et nécessaires pour vous dans n'importe quel processus, il peut être utile de commencer par supprimer celles qui sont manifestement inutiles et qui nuisent à la culture d'ingénierie. Alors faisons-le.

Partage de connaissances

À la question «Pourquoi ai-je besoin d'un processus de révision de code?» J'ai entendu à plusieurs reprises la réponse " Partager les connaissances ". En effet, une chose utile et nécessaire. Et beaucoup conviennent qu'il est important d'avoir un processus dans l'équipe pour l'échange de connaissances et d'expertise. Mais êtes-vous sûr que la révision du code est appropriée pour cela?


J'ai demandé à plusieurs reprises aux gens ce qu'ils entendaient par «partage des connaissances». Et en réponse, j'ai entendu différentes choses.

Premièrement, il s'agit d'une démonstration aux nouveaux arrivants (dans l'équipe ou dans la composante concernée) des règles et pratiques acceptées: c'est ainsi que nous avons l'habitude de le faire, et nous ne le faisons pas, c'est pourquoi c'est le cas.

Deuxièmement, il s'agit d'un nouveau regard d'un débutant (encore une fois dans une équipe ou un composant affecté) sur la façon dont les choses ordinaires sont faites. Tout à coup, ils sont inefficaces, et une nouvelle personne aidera-t-elle à découvrir cela?

Troisièmement, ce n'est qu'une introduction à une partie du code. En effet, si à l'avenir le réviseur est confronté à la nécessité de modifier cette partie du code, cela lui sera beaucoup plus facile.

Passons en revue tous les points et essayons d'évaluer leur pertinence dans le processus d'examen.

La révision du code comme outil de formation pour les débutants

Dans ce cas, nous parlons principalement d'un tel scénario: un débutant reçoit une tâche, il l'exécute, puis un membre expérimenté de l'équipe (propriétaire du composant) indique les erreurs qu'il a faites. Le processus est important et nécessaire, je ne conteste pas - les débutants doivent en quelque sorte montrer les règles acceptées dans l'équipe.

Mais soyons honnêtes: n'est-il pas trop tard? Ne serait-il pas plus juste de parler de ces règles au débutant avant de l' admettre au code? Après tout, le prix d'une erreur est très élevé - rarement les débutants travaillent immédiatement comme vous le souhaitez. Ainsi, la tâche reviendra et reviendra pour révision. Ainsi, le produit deviendra disponible à l'utilisateur plus tard qu'il ne le pourrait.

Il s'avère que dans un processus dont la durée est difficile à évaluer, nous en ajoutons un autre dont les coûts en temps sont encore moins prévisibles. Ainsi, nous augmentons le temps nécessaire pour livrer la tâche à l'utilisateur d'une quantité encore plus inconnue.

Bien sûr, parfois la formation des débutants à travers les processus de travail a le droit d'exister. Mais parfois, nous ne pensons pas aux lacunes des approches qui sont devenues une habitude. Et faire une erreur ici n'est pas seulement facile, mais très facile.

Par exemple, si l'entreprise n'a pas de processus rationalisé de formation et de coaching , le manager est obligé de «jeter à l'eau» un nouvel arrivant. Dans le même temps, ce dernier n'a pas le choix: il faut soit "nager", soit quitter l'entreprise. Quelqu'un apprend vraiment de telles situations et quelqu'un ne se lève pas. Je pense que beaucoup ont rencontré des problèmes similaires tout au long de leur parcours professionnel. Mon message est que le temps le plus précieux peut être dépensé de manière non optimale à cause de ce phénomène. En plus du processus d'adaptation d'un nouvel employé dans une équipe, il peut ne pas être optimal. Tout simplement parce que personne n'avait les mains pour organiser un processus efficace d'introduction de nouveaux employés dans l'équipe, et le gestionnaire pensait que le nouveau venu apprendrait tout au stade de la révision du code. Mais en fait, ce sont deux processus différents, très nécessaires et importants.

Bien sûr, même dans les conditions où les règles ont été initialement expliquées au débutant et que l'entreprise a d'autres processus de formation, le processus d'adaptation du débutant doit être contrôlé d'une manière ou d'une autre. Un examen est un processus qui peut vous aider. Mais le contrôle et l'entraînement sont deux choses différentes, non? De plus, tous les membres de l'équipe ont besoin de contrôle, et pas seulement les débutants. Après tout, nous pouvons tous faire des erreurs, oublier les accords et affecter d'une manière ou d'une autre la partie du système que toute l'équipe possède (dans ce cas, le code).

Quelle conclusion peut-on tirer? Le processus décrit ci-dessus vise à atteindre un objectif différent: non pas pour la formation, mais pour le contrôle. Et ce même contrôle est nécessaire non seulement pour les débutants, mais pour l'équipe dans son ensemble.

Tout cela est facilement formulé dans la thèse suivante: "Une révision du code est nécessaire pour contrôler le respect des accords et des règles générales par tous les membres de l'équipe ." Et la formation des débutants dans ce cas ne sera rien de plus qu'une substitution artificielle d'un objectif qui ne fera que confondre les gens et diriger le processus dans une direction différente.

Mais même avec cette conclusion, qui, semble-t-il, a été formulée correctement, le bois de chauffage peut être cassé. Une révision de code est une phase qui commence lorsque le code est déjà écrit. Et il peut arriver que le code écrit sans suivre les règles générales soit très coûteux à personnaliser selon le modèle général. Notre tâche est d'informer l'entrepreneur que quelque chose s'est mal passé le plus tôt possible. Et par conséquent, je soutiens que, parfois, les révisions de code ne sont pas le processus le plus approprié pour contrôler le respect des règles générales. Dans de nombreux cas, les contrôles de conformité peuvent et doivent être transférés à des stades antérieurs.

Par exemple, vous pouvez vérifier la mise en forme du code dans les crochets du système de contrôle de version (ainsi que l'utilisation de classes sécurisées, les dispositions pour l'emplacement des fichiers dans les dossiers correspondants, l'utilisation de dépendances externes et de bibliothèques tierces, etc.). Cela minimise le temps requis pour la révision du code.

Poursuivant la conversation sur la formation des débutants au processus de révision du code, dessinons une analogie. Supposons que vous produisiez une sorte de produit, par exemple des gâteaux. Supposons que vous ayez reçu une commande de gâteau pour un mariage VIP. Confiez-vous la «révision» du gâteau fini au débutant? Êtes-vous prêt à ce que le nouveau confiseur assume la honte ou le succès de toute la boulangerie à ce mariage? Ou peut-être voulez-vous qu'il se familiarise à ce stade avec les règles adoptées par l'équipe (comment faire des gâteaux, préparer de la crème et insister sur les canneberges sur le cognac)? De toute évidence, à la fois le processus d'introduction du débutant aux règles et le contrôle de sa part sont des choses plutôt étranges à ce stade: le gâteau a déjà été cuit. Alors, pourquoi pouvons-nous agir différemment avec les fonctionnalités et le code? Ou les fonctionnalités que nous lançons n'apportent ni honte ni succès à notre équipe aux yeux des utilisateurs et des clients?

Vous pouvez vous opposer, en disant que nous ne préparons pas des tâches «pour des personnes importantes» tous les jours et qu'il est tout à fait possible pour un débutant de se voir confier une tâche peu urgente ou peu importante. Et vous aurez raison.

Mais tout d'abord, qu'est-ce qu'un débutant apprend des tâches triviales dans lesquelles il y a peu de changements de code (et ces changements ne sont pas dans des endroits critiques et pour les entreprises ne sont probablement pas si importants, car la tâche est urgente)? Comment peut-il apprendre à faire des gâteaux s'il fouette la crème tout le temps? La transition du simple au complexe est bien sûr nécessaire. Mais il est nécessaire de le faire, en contrôlant soigneusement le processus. Il faut enseigner aux débutants et non pas arrêter d'apprendre par eux-mêmes.

Et deuxièmement, même une telle approche apparemment utile et correcte peut nuire à une entreprise. Pour comprendre cela est très simple, en utilisant la méthode d' amener au point d'absurdité pour rendre la conclusion aussi simple et compréhensible que possible.

Imaginez que nous déclarions ouvertement que les tâches que nous confie le chef de produit sont nécessaires à la formation des nouveaux arrivants. Pourquoi? Et la même chose que pour la révision du code: après tout, nous confions des tâches simples et non urgentes aux débutants afin qu'ils apprennent à travailler de la manière habituelle chez nous.

Que peut-il en résulter à la fin? Un interprète zélé qui fait fidèlement son travail et croit que tout ce qu'il fait doit être fait le mieux possible, reprendra le processus d'apprentissage. Il peut définir la tâche pour effectuer plusieurs implémentations au lieu d'une, afin que plus tard, il puisse montrer à l'apprenant les inconvénients et les avantages des différentes approches. Il peut donner des conférences, comparer les approches et les meilleures pratiques utilisées dans l'entreprise et au-delà. Il peut faire beaucoup plus pour éduquer le débutant. En conséquence, le temps requis pour terminer la tâche augmentera, car au lieu de se développer, nous nous concentrons sur la formation .

Dans le cas d'une révision de code, un tel employé zélé lance une longue discussion dans les commentaires de la révision sur les avantages et les dangers de certaines implémentations, publie des morceaux de code avec Stack Overflow, montrant qu'il y a d'autres idées dans d'autres têtes, donne des liens vers des articles et des livres que l'étudiant devrait lire pour comprendre que sa mise en œuvre est imparfaite.

Il s'agit d'un processus d'apprentissage normal qui peut exister, mais qui doit être fait correctement. Et il est loin d'être toujours utile de l'intégrer dans le processus de résolution d'un problème commercial. Parce que ce sont deux processus différents. Laissez le débutant faire les différents composants du gâteau, laissez-le faire plusieurs options, laissez quelque chose se ruiner et jetez-le. Laissez quelqu'un de plus expérimenté lui montrer comment agir et raconter de vieilles recettes. Mais apprendre «sur la chaîne de production» qui produit votre produit pour les clients n'est pas une bonne idée. Et si vous avez déjà décidé de cela, alors «menez le nouveau venu» par la poignée, ne lui permettant pas d'interférer avec le processus de production pendant sa formation.

Si votre entreprise n'est pas une institution, ni une école, ni une école technique ou tout autre établissement d'enseignement, la formation n'est pas de votre responsabilité directe. L'entreprise vous a engagé pour résoudre d'autres problèmes et obtenir d'autres résultats.

Soit dit en passant, c'est pourquoi nous n'ajoutons pas de fonctionnalité à Codeisok pour télécharger des images, taper, mettre en surbrillance le code dans les commentaires, bien qu'il y ait eu et qu'il y ait encore de nombreuses demandes pour de telles fonctionnalités. Nous promouvons l'idée que la révision de code n'est pas un blog personnel, ni un messager, ni un service de formation. Un outil est nécessaire pour un autre. En effet, pour signaler les failles du code, un commentaire en clair est plus que suffisant.

La révision du code comme un nouveau regard sur le code

Continuons. Le scénario suivant «d'échange d'expérience» est le suivant: nous faisons quelque chose dans l'équipe, observons les accords internes, et nos yeux sont flous, puis une nouvelle personne est venue (d'une autre entreprise ou d'une autre composante) - et peut-être qu'il verra des défauts dans notre organisation du travail.

L'idée est bonne, cela semble raisonnable. Et si nous faisons quelque chose de mal?

Mais encore une fois, revenons à la vie de la tâche et au début de la phase de révision du code. Est-ce trop tard? Le gâteau est déjà cuit, les gâteaux sont enduits de crème, les roses sont collées. Le prix de l'erreur est très élevé. Et puis nous découvrons que dans une autre boulangerie dans les gâteaux ajouter du soda, recouvert de jus de citron, pour plus de splendeur. Et alors? Que faire Remodeler?

Évidemment non, car: 1) nous nous sommes toujours débarrassés du soda, et le résultat n'était pas mauvais; 2) il est possible que les clients nous prennent des gâteaux, et non d'une autre boulangerie, parce qu'ils n'aiment pas le goût du soda; 3) , , .

? . . , , . , , : « ? ?» , , - -, .

best practices, , . « : „“, „“, „.“, „“. ». Et alors? - , ?

, . , , , « », . « ». , , .

, — , . , , — . - . . - . , , , .

. , . «Code is OK». , , , .

Code review

, , . : , , — , . bus factor : , . , «» - , , .

, , , .


, . , ?

, - (, , , ). , ? , . , , , .

, , , , . . , ?

, - . , - . , : , . , , , , .

, . , - ? — . , , .

, , , - . , . , , . , , , . . , - : , , .

? , , — . .

best practices « -» « , ». Et alors? Best practices , , . , ? . , - , .

best practices, , . . — - , / — . « », « », « — , DI». ? ?

, , . , , — , .

, -, «», , - . ? - , , , . , . , .

-, « » «» , . -, . , , - , : ? . , , ( : , ).

, , . , , ( , ?).

— . , , . . ? bus factor , , - , .

, , , : , ?

— , , ?
— .
— ?
— .

. , ? , , , ? , , , — ?

, , , , , ?

, , , . , , .

, . , , , ( ), , ( , , , — , . .). , .

— , , , . ( , , . .) . — — .

Code review

« ?» . , , .

: - , , - , , . git blame , , , , , .

, , — , , , ( ) — . , , . . , ?


. ?

, , , . , , , . , - ( , ). , , — , , , .

, , . ? , .

, , , . , . , ? .

, , — , . ( )?

« - - » . , . .

: . .

, , . , . , .

, , — . , .

— . , . , . , .

, , :

? , , ? ? , , ? - , ? ? ? Et ainsi de suite.

, . , . , .

, , . , , , . , , , .

, best practices, . , , , , . . , , . , .

,

. , , . - , , - .

, , . , API- . , bus factor, .

, . .

- ? , . , . null , « ». .

, . , ? ?

-

Cette étape des vérifications est également très importante, car elle vise à améliorer le support du code notoire.

Beaucoup de gens comprennent ce que signifie le terme maintenabilité du code, mais tout le monde ne peut pas l'expliquer. Et si oui, comment alors confier la tâche à l'équipe pour maintenir cette maintenabilité? Par conséquent, je crois qu'un développeur qui réfléchit à la façon dont son code sera testé, sans parler de tester son propre code et d'écrire un autotest pour automatiser les tests, s'efforcera naturellement de s'assurer que son code satisfait la plupart des critères de maintenabilité du code. En effet, sans cela, écrire un autotest est presque impossible.

La recommandation générale pour tout changement de code peut également être la décomposition correcte des tâches . Plus la quantité de code passant par le pipeline de tous les processus, de l'idée à la production, est réduite, plus ce code est facile à vérifier, à tester, à combiner avec le code d'autres tâches et à télécharger. Une révision de code ne fait pas exception. Une tâche avec un grand nombre de lignes modifiées dans de nombreux fichiers est très difficile à lire et à comprendre (et à garder à l'esprit les dépendances). Le risque et le coût de la correction des bogues peuvent être très élevés.

Conclusion

En fait, c'est tout. Ces quatre points sont précisément les choses mêmes qui doivent être vérifiées au stade de l'examen. Ils sont faciles à formaliser, à transmettre aux interprètes, ce qui signifie qu'il n'est pas difficile de formuler des critères de vérification et de prévoir la durée. L'automatisation et d'autres moyens de minimiser le temps d'examen sont plus que possibles.

Et enfin, je vais vous donner quelques conseils supplémentaires.

Il est important de se rappeler que tout retour du code pour révision n'affecte pas la qualité de l'implémentation de la meilleure façon. Même si tout le monde est bien intentionné. Dans le cas de la révision de code, cela fonctionne comme ceci: le développeur corrige le code, mais ne le teste pas aussi complètement que lors de la première itération (pourquoi? Après tout, il a déjà tout vérifié et l'a juste changé un peu). L'expérience a montré que la plupart de ces situations se transforment en bogues précisément aux endroits où il y a eu des corrections basées sur les résultats de la révision du code. C'est ainsi que fonctionne la psychologie humaine.

Au début de l'article, je vous ai suggéré de répondre à trois questions sur la nécessité et l'exactitude d'une revue. Vous disposez maintenant d'un algorithme pour vous aider à trouver les réponses. Connaissant les étapes de contrôle et tenant compte de la taille de la tâche, il est tout à fait possible de planifier le temps nécessaire à la révision du code, chaque fois en essayant de le réduire de plus en plus.

Lors de la mise en œuvre de tout processus, il est important de se souvenir des objectifs que nous nous sommes fixés et des problèmes que nous voulons résoudre. Il sera alors beaucoup plus facile de séparer les grains de l'ivraie et de formuler des critères mesurables pour évaluer l'efficacité. Et vous devez les formuler pour vous-même et pour votre équipe, qui a également un besoin urgent de résoudre les problèmes qui se sont posés, mais peut les comprendre à sa manière.

Il y a un autre point important: les processus, les règles et les valeurs qui sont justes pour une entreprise peuvent ne pas être aussi utiles et fonctionner dans une autre. Ce que j'ai décrit comme des exemples de l'examen correct fonctionne dans notre entreprise. Nous favorisons un développement rapide, des versions fréquentes et une révision pour chaque tâche. Réfléchissez à la façon dont cela est applicable à votre projet, et qu'un examen est l'un de ces processus qui ne peut être laissé au hasard.

Mais la décision, bien sûr, vous appartient.