À partir de la documentation des pratiques d'ingénierie de Google

Ce guide fournit les meilleures pratiques pour effectuer des révisions de code basées sur des années d'expérience. Ensemble, ils constituent un document, divisé en plusieurs sections. Il n'est pas nécessaire de les lire tous, mais souvent pour vous et votre équipe, il est préférable d'étudier le guide en entier.

• Norme de révision du code
  
• Quoi enregistrer dans le code
  
• Navigation dans la liste des modifications (CL)
  
• Vitesse de révision du code
  
• Comment rédiger des commentaires
  
• Comment surmonter la résistance

Consultez également le Guide de l'auteur CL pour obtenir des conseils détaillés sur les développeurs dont les validations sont examinées.

Norme de révision du code

L'objectif principal d'une révision de code est de garantir une amélioration continue de la base de code Google. Tous les outils et processus sont dédiés à cet objectif.

Un certain nombre de compromis sont nécessaires ici.

Premièrement, les développeurs devraient être en mesure de résoudre avec succès leurs problèmes . Si vous n'envoyez jamais de code, la base de code ne s'améliorera jamais. De plus, si le réviseur complique grandement tout travail, à l'avenir, les développeurs ne sont pas intéressés à proposer des améliorations.

D'autre part, il est de la responsabilité de l'examinateur de s'assurer que la qualité du CL ne diminue pas la qualité globale de la base de code au fil du temps. Cela peut être difficile, car souvent une dégradation se produit en raison d'une légère diminution de la qualité du code au fil du temps, surtout si l'équipe est soumise à une forte pression des délais et estime qu'elle a le droit d'augmenter la dette technique.

De plus, le réviseur est responsable du code en cours de révision. Il veut s'assurer que la base de code reste cohérente, prise en charge et correspond à tout le reste qui est mentionné dans la section «Quoi archiver le code» .

Ainsi, nous obtenons la règle suivante comme norme pour la révision de code:

En règle générale, les examinateurs devraient approuver le CL dès qu'il atteint un état où il améliore définitivement la qualité globale du code système, même si le CL n'est pas parfait.

Il s'agit de la principale révision du code parmi tous les principes.

Bien sûr, il a ses limites. Par exemple, si CL ajoute une fonction que le réviseur ne veut pas voir dans le système, le réviseur peut certainement refuser de valider, même si le code est de bonne qualité.

Le point clé ici est qu'il n'y a pas de code «parfait» - il n'y a qu'un meilleur code. L'examinateur ne devrait pas exiger de l'auteur qu'il peaufine chaque petit fragment. L'examinateur devrait plutôt trouver un équilibre entre la nécessité de progresser davantage et l'importance des changements proposés. Au lieu de viser l'idéal, l'examinateur devrait s'efforcer d' améliorer continuellement . Un commit, qui améliore généralement la maintenabilité, la lisibilité et l'intelligibilité du système, ne peut pas être retardé pendant des jours ou des semaines car il n'est pas "parfait".

Les réviseurs peuvent toujours laisser des commentaires sur l'amélioration du code, mais les modifications peu importantes doivent être marquées, par exemple, avec le préfixe Nit: afin que l'auteur sache que ce n'est qu'un point de vue qu'il peut ignorer.

Remarque Rien dans ce document ne justifie les CL qui dégradent définitivement la qualité globale du code système. Cela n'est possible qu'en cas d' urgence .

Le mentorat

Une révision de code peut également être importante pour enseigner aux développeurs quelque chose de nouveau sur le langage, la structure ou les principes généraux de la conception de logiciels. Il est toujours agréable de laisser des commentaires qui aident le développeur à apprendre quelque chose de nouveau. Le partage des connaissances contribue à améliorer le code système au fil du temps. Gardez à l'esprit que si vous laissez un commentaire purement éducatif qui n'est pas essentiel pour répondre aux normes décrites ici, ajoutez-y le préfixe Nit: ou indiquez autrement que l'auteur n'est pas tenu de le permettre.

Principes

• Les faits et données techniques l'emportent sur les opinions et les préférences personnelles.
  
• En matière de style, l'autorité absolue est le guide du style . Tout détail purement stylistique (espace, etc.) qui n'est pas inclus dans le guide de style est une question de préférence personnelle. Le style doit correspondre à ce qu'il est. S'il n'y a pas de style précédent, acceptez l'auteur.
  
• Les aspects de la conception de logiciels ne sont presque jamais une question de style pur ou de préférence personnelle. Ils sont basés sur des principes fondamentaux et devraient être déterminés par ces principes, et pas seulement par une opinion personnelle. Parfois, il existe plusieurs options valides. Si l'auteur peut démontrer (en utilisant des données ou sur la base de principes d'ingénierie solides) que certaines approches sont tout aussi efficaces, le réviseur doit accepter la préférence de l'auteur. Sinon, le choix est dicté par les principes de développement standard.
  
• Si aucune autre règle n'est applicable, le réviseur peut demander à l'auteur de respecter l'uniformité avec la base de code actuelle, si cela n'aggrave pas l'état général du système.

Résolution des conflits

Dans tout conflit, la première étape doit toujours être le désir du développeur et du réviseur de parvenir à un consensus sur la base du contenu de ce document et d'autres documents dans le Guide de l'auteur du CL et ce Guide du réviseur .

Lorsqu'il est particulièrement difficile de parvenir à un consensus, une réunion personnelle ou une vidéoconférence entre le réviseur et l'auteur peut aider (si vous le faites, assurez-vous d'écrire les résultats de la discussion dans un commentaire à valider pour les futurs lecteurs).

Si cela ne résout pas la situation, le moyen le plus courant est l'escalade. Cela consiste souvent en une discussion plus large avec l'équipe, en attirant le chef d'équipe, en contactant le responsable ou le responsable du développement. Ne laissez pas le commit s'attarder car l'auteur et le réviseur ne peuvent pas parvenir à un accord.

Quoi enregistrer dans le code

Remarque Lors de l'examen de chacun de ces éléments, assurez-vous de considérer la révision du code standard .

La conception

La chose la plus importante est de considérer le projet global (conception) dans la révision du code. Les interactions entre les différentes parties du code ont-elles un sens? Cette modification s'applique-t-elle à votre base de code ou à votre bibliothèque? CL s'intègre-t-il bien avec le reste du système? Est-il temps d'ajouter cette fonctionnalité maintenant?

Fonctionnalité

Ce CL fait-il ce que le développeur voulait? Est-ce bon pour les utilisateurs de ce code? Par «utilisateurs», nous entendons à la fois les utilisateurs finaux (s'ils sont affectés par le changement) et les développeurs (qui devront «utiliser» ce code à l'avenir).

Fondamentalement, nous nous attendons à ce qu'avant même de s'engager, les développeurs testent suffisamment leur code pour qu'il fonctionne correctement. Mais en tant que réviseur, vous devez toujours penser aux cas extrêmes, rechercher les problèmes de concurrence, essayer de penser en tant qu'utilisateur et même regarder en lisant le code qu'il n'y a pas d'erreurs évidentes.

Si vous le souhaitez, vous pouvez vérifier les performances. Ceci est plus important si le code affecte les utilisateurs, comme la modification de l'interface utilisateur . Il est difficile de comprendre comment certains changements affecteront les utilisateurs lorsque vous lirez le code. Pour de tels changements, vous pouvez demander au développeur de fournir une démo s'il vous est trop difficile de vous plonger dans le code et de l'essayer vous-même.

Un autre point où il est particulièrement important de penser aux fonctionnalités lors d'une révision de code est de savoir si une sorte de programmation parallèle se produit dans le CL, ce qui peut théoriquement provoquer des blocages ou des conditions de concurrence. De tels problèmes sont très difficiles à détecter en exécutant simplement le code; généralement, il est nécessaire que quelqu'un (le développeur et le réviseur) réfléchisse soigneusement à eux et s'assure qu'aucun problème n'est introduit (notez que c'est également une bonne raison de ne pas utiliser de modèles de parallélisme où les conditions de concurrence ou les blocages sont possibles - cela peut être fait par code très difficile à comprendre ou à revoir le code).

Difficulté

Le CL est-il plus compliqué qu'il ne devrait l'être? Vérifiez-le à tous les niveaux: lignes distinctes, fonctions, classes. Une «complexité excessive» signifie généralement l' incapacité de comprendre rapidement pendant la lecture . Cela peut également signifier que les développeurs sont plus susceptibles d'introduire des erreurs lorsqu'ils tentent d'appeler ou de modifier ce code .

Un type particulier de complexité est la sur-ingénierie, lorsque les développeurs ont rendu le code plus universel qu'il ne devrait l'être, ou ajouté des fonctionnalités dont le système n'a pas besoin actuellement. Les examinateurs doivent être particulièrement vigilants concernant la suringénierie. Encouragez les développeurs à résoudre un problème qui devrait certainement être résolu maintenant, plutôt qu'un problème qui pourrait devoir être résolu à l'avenir. Un problème futur devrait être résolu lorsqu'il apparaît, et vous pouvez voir sa forme réelle et ses exigences dans l'Univers physique.

Les tests

Demandez des tests unitaires, d'intégration ou de bout en bout pertinents pour le changement. En général, les tests doivent être ajoutés au même CL que le code de production si le CL ne gère pas l' urgence .

Assurez-vous que les tests sont corrects, raisonnables et utiles. Les tests ne se testent pas eux-mêmes, et nous écrivons rarement des tests pour nos tests - une personne doit s'assurer que les tests sont valides.

Les tests échouent-ils vraiment sur du code cassé? Si le code change, y aura-t-il des faux positifs? Chaque test fait-il des déclarations simples et utiles? Les tests sont-ils correctement répartis entre différentes méthodes de test?

N'oubliez pas que les tests sont du code qui doit également être pris en charge. Ne leur permettez pas la complexité simplement parce qu'elle ne fait pas partie du fichier binaire principal.

Nommer

Le développeur a-t-il choisi de bons noms partout? Un bon nom est suffisamment long pour exprimer pleinement ce qu'est l'élément ou ce qu'il fait, sans être si long qu'il devient difficile à lire.

Commentaires

Le développeur a-t-il écrit des commentaires clairs dans un langage simple? Tous les commentaires sont-ils nécessaires? Les commentaires sont généralement utiles lorsqu'ils expliquent pourquoi un code existe et ne doivent pas expliquer ce que fait ce code. Si le code n'est pas suffisamment clair pour s'expliquer, il doit être simplifié. Il existe quelques exceptions (par exemple, les commentaires expliquant les actions du code sont souvent très utiles pour les expressions régulières et les algorithmes complexes), mais la plupart du temps, les commentaires sont destinés à des informations que le code lui-même ne peut pas contenir, par exemple, pour étayer une décision.

Il peut également être utile de consulter les commentaires du code précédent. Il y a peut-être un TODO, qui peut maintenant être supprimé, ou un commentaire qui ne recommande pas l'introduction de cette modification, etc.

Notez que les commentaires diffèrent de la documentation des classes, modules ou fonctions, qui décrit la tâche du code, comment il doit être utilisé et comment il se comporte.

Le style

Nous avons des guides de style Google pour toutes les langues principales, et même la plupart des langues mineures. Assurez-vous que le CL n'est pas en conflit avec les guides de style pertinents.

Si vous souhaitez améliorer certains éléments qui ne figurent pas dans le guide de style, ajoutez une note au commentaire ( Nit :) . Le développeur saura que c'est votre remarque personnelle, qui n'est pas contraignante. Ne bloquez pas l'envoi de commits uniquement en fonction de vos préférences personnelles.

L'auteur ne doit pas combiner des changements de style importants avec d'autres changements. Cela rend difficile de voir les changements dans le CL, complique les fusions, les annulations de code et provoque d'autres problèmes. Par exemple, si l'auteur souhaite reformater l'intégralité du fichier, demandez un changement de style dans un CL, puis envoyez le CL avec des changements fonctionnels.

La documentation

Si la sortie CL change quelque chose dans l'assemblage, les tests, les procédures d'interaction ou la version de code, recherchez les mises à jour de la documentation pertinente, y compris les fichiers README, les pages g3doc et tous les documents de référence générés. Si le CL supprime le code ou le rend obsolète, envisagez de supprimer également la documentation. Si la documentation est manquante, demandez sa création.

Chaque rangée

Affichez chaque ligne du code. Bien que tous les fichiers de données, le code généré ou les grandes structures de données puissent être brièvement examinés, mais lisez attentivement chaque classe, fonction ou bloc de code écrit par une personne, ne supposez jamais par défaut que tout est en ordre. De toute évidence, un code mérite une étude plus approfondie qu'un autre - vous décidez par vous-même, mais vous devez au moins être sûr de comprendre le fonctionnement de tout le code.

S'il vous est trop difficile de lire le code et que cela ralentit la révision, vous devez en informer le développeur et attendre qu'il apporte de la clarté avant de continuer. Chez Google, nous embauchons d'excellents ingénieurs logiciels, et vous êtes l'un d'entre eux. Si vous ne comprenez pas le code, il est très probable que d'autres développeurs ne pourront pas le faire. De cette façon, vous aidez également les futurs développeurs à comprendre ce code en leur demandant de la clarté.

Si le code est compréhensible, mais que vous ne vous sentez pas suffisamment qualifié pour évaluer un certain fragment, assurez-vous qu'un réviseur dans le CL est qualifié, en particulier pour les problèmes complexes tels que la sécurité, la concurrence, l'accessibilité, l'internationalisation, etc.

Contexte

Il est souvent utile d'examiner le CL dans un contexte large. En règle générale, un outil de révision de code n'affiche que quelques lignes près du lieu de modification. Parfois, vous devez consulter l'intégralité du fichier pour vous assurer que le changement a vraiment du sens. Par exemple, vous voyez l'ajout de seulement quatre lignes, mais si vous regardez le fichier entier, alors ces quatre lignes sont dans la méthode de 50 lignes, qui doit maintenant vraiment être divisée en plus petites.

Il est également utile de penser à CL dans le contexte du système dans son ensemble. Améliore-t-il la qualité du code système ou le rend-il plus complexe, moins testé, etc.? N'acceptez pas les validations qui dégradent le code système. La plupart des systèmes sont compliqués par la somme de nombreux petits changements, il est donc important d’y éviter même des difficultés mineures.

Bon

Si vous voyez quelque chose de bien dans le commit, faites-le savoir au développeur, surtout lorsqu'il a résolu le problème décrit dans l'un de vos commentaires de la meilleure façon possible. Les révisions de code se concentrent souvent simplement sur les bogues, mais elles devraient également encourager et valoriser les bonnes pratiques. Du point de vue du mentorat, il est parfois encore plus important de dire au développeur ce qu'il a fait correctement, et non où il a fait une erreur.

Résumé

Lors de l'exécution d'une révision de code, assurez-vous que:

• Le code est bien conçu.
  
• La fonctionnalité est bonne pour les utilisateurs de code.
  
• Toutes les modifications apportées à l'interface utilisateur sont raisonnables et semblent bonnes.
  
• Toute programmation simultanée est sûre.
  
• Le code n'est pas plus compliqué qu'il ne devrait l'être.
  
• Le développeur n'implémente pas ce qui pourrait être nécessaire à l'avenir avec des perspectives inconnues.
  
• Le code est bordé de tests unitaires appropriés.
  
• Les tests sont bien conçus.
  
• Le développeur a utilisé des noms clairs partout.
  
• Les commentaires sont compréhensibles et utiles, et répondent essentiellement à la question pourquoi? mais pas quoi?
  
• Le code est correctement documenté (généralement dans g3doc).
  
• Le code est conforme à nos guides de style.

Pendant l'examen, assurez-vous de regarder chaque ligne de code, regardez le contexte , assurez-vous que vous améliorez la qualité du code et félicitez les développeurs pour le bien qu'ils ont réussi à faire.

Navigation CL dans la revue de code

Résumé

Maintenant que vous savez comment archiver le code , quel est le moyen le plus efficace d'effectuer des révisions de code sur plusieurs fichiers?

1. Ce changement est-il logique? At-il une bonne description ?
   
2. Regardez d'abord la partie la plus importante. Est-il bien conçu?
   
3. Regardez le reste du CL dans la séquence appropriée.

Première étape: jetez un œil à l'ensemble du commit

Regardez la description CL et ce qu'elle fait en général. Ce changement est-il logique du tout? S'il n'a pas dû être rédigé initialement, veuillez répondre immédiatement en expliquant pourquoi il n'est pas nécessaire. Lorsque vous refusez un tel changement, il est également agréable de proposer au développeur ce qu'il doit faire à la place.

Par exemple, vous pourriez dire: «On dirait que vous avez fait du bon travail, merci!» Cependant, nous allons supprimer le système FooWidget, nous ne voulons donc pas apporter de nouvelles modifications pour le moment. Que diriez-vous de refactoriser notre nouvelle classe BarWidget à la place? "

Veuillez noter que l'examinateur a non seulement rejeté le CL actuel et fourni une autre suggestion, mais l'a également fait poliment . Cette courtoisie est importante parce que nous voulons montrer que nous nous respectons en tant que développeurs, même lorsque nous sommes en désaccord les uns avec les autres.

Si vous obtenez un certain nombre de CL indésirables, vous devriez revoir le processus de développement de votre équipe ou les règles publiées pour les contributeurs externes afin d'améliorer la communication lors de la rédaction du CL. Il vaut mieux dire «non» aux gens avant de faire une tonne de travail qui devra être jeté ou réécrit lourdement.

Deuxième étape: apprendre les éléments de base du CL

Recherchez le ou les fichiers qui représentent la partie "principale" de ce CL. Il y a souvent un fichier avec les changements les plus logiques, et c'est la partie principale du CL. Regardez d'abord ces parties principales. Cela permet de comprendre le contexte des petites parties du CL et accélère généralement l'exécution de la révision du code. Si le CL est trop volumineux, demandez au développeur ce qu'il doit voir en premier ou demandez-lui de diviser le CL en plusieurs parties .

Si vous voyez de graves problèmes dans la partie principale du CL, vous devez immédiatement envoyer ces commentaires, même si vous n'avez pas le temps de consulter le reste du CL pour le moment. En fait, afficher le reste du code peut être une perte de temps, car si les problèmes sont suffisamment importants, de nombreux autres morceaux de code en question disparaîtront et n'auront plus d'importance.

Il y a deux raisons principales pour lesquelles il est si important d'envoyer ces principaux commentaires immédiatement:

• Les développeurs envoient souvent CL, puis démarrent immédiatement un nouveau travail en fonction de celui-ci, en attendant le résultat d'une révision de code. Si le CL a de sérieux problèmes, il devra retravailler le prochain CL. Il est conseillé d'identifier les erreurs le plus tôt possible avant qu'elles ne fassent beaucoup de travail supplémentaire en plus de la conception problématique.
  
• Les modifications majeures prennent plus de temps que les modifications mineures. Presque tous les développeurs ont des délais. Afin de rester en eux et de ne pas réduire la qualité du code, toute refactorisation majeure devrait débuter dans les meilleurs délais.

Étape trois: faites défiler le reste du CL dans la séquence appropriée.

Après vous être assuré qu'il n'y a pas de problème sérieux avec la conception du CL dans son ensemble, essayez de comprendre la séquence logique pour visualiser les fichiers et assurez-vous que rien ne manque. Habituellement, lorsque vous regardez les fichiers principaux, le moyen le plus simple consiste à simplement parcourir les autres dans l'ordre dans lequel l'outil de révision du code les présente. Il est également parfois utile de lire d'abord les tests, puis le code principal, car vous aurez alors une idée de la signification du changement.

Vitesse de révision du code

Pourquoi une révision du code devrait-elle être effectuée rapidement?

Chez Google, nous optimisons la vitesse de collaboration , pas la vitesse des développeurs individuels. La vitesse du travail individuel est importante, elle n'est tout simplement pas aussi prioritaire par rapport à la vitesse du travail d'équipe.

Lorsque vous regardez lentement le code, plusieurs choses se produisent:

• La vitesse de l'équipe dans son ensemble diminue. Oui, si une personne ne répond pas rapidement à une révision de code, elle fait un autre travail. Cependant, pour le reste de l'équipe, les nouvelles fonctionnalités et les corrections de bugs sont retardées de plusieurs jours, semaines ou mois, car chaque CL attend une révision de code et une révision de code répétée.
  
• -. , , . , «» . (, ), , . - .
  
• La qualité du code peut en souffrir. Le ralentissement de la révision du code augmente le risque que les développeurs n'envoient pas des CL aussi bons qu'ils pourraient l'être. Les révisions lentes entravent également le nettoyage du code, la refactorisation et d'autres améliorations des CL existants.

Comment exécuter rapidement une revue de code?

Si vous n'êtes pas au milieu d'une tâche ciblée, vous devez créer un code de révision peu de temps après son arrivée.

Un jour ouvrable est le temps maximum pour une réponse (c'est-à-dire que c'est la première chose le lendemain matin).

Le respect de ces directives signifie qu'un CL type devrait recevoir plusieurs séries de révision (si nécessaire) en une journée.

Vitesse et distraction

Il y a un moment où la priorité de la vitesse personnelle dépasse la vitesse de l'équipe. Si vous êtes au milieu d'une tâche ciblée, comme l'écriture de code, ne vous laissez pas distraire par la révision du code. Des études ont montré qu'un développeur peut mettre du temps à retrouver un flux de développement fluide après une distraction. Ainsi, la distraction du codage coûte en fait plus cher à l'équipe que de demander à un autre développeur d'attendre un peu la révision du code.

Attendez plutôt un point d'arrêt dans votre travail. Par exemple, après avoir terminé la tâche en cours, après le déjeuner, en revenant d'une réunion, en revenant d'une kitchenette de bureau, etc.

Réponses rapides

Lorsque nous parlons de la vitesse de révision du code, nous nous intéressons au temps de réponse , et non au temps nécessaire à l'ensemble du processus jusqu'à la fin. Idéalement, l'ensemble du processus devrait également être rapide, mais il est encore plus important que les réponses individuelles arrivent rapidement que l'ensemble du processus .

Même si l'ensemble du processus de révision du code prend du temps, la disponibilité de réponses rapides du réviseur tout au long du processus simplifie considérablement la vie des développeurs qui peuvent être ennuyés par des réponses «lentes».

Si vous êtes trop occupé pour un examen complet immédiatement après avoir reçu la demande, vous pouvez toujours répondre rapidement avec un message sur les délais ou proposer l'examen à d'autres examinateurs qui peuvent répondre plus rapidement, oufournissez quelques premiers commentaires généraux (note: rien de tout cela ne signifie que vous devez interrompre le codage même pour envoyer une telle réponse - envoyez la réponse à un point d'arrêt raisonnable dans votre travail).

Il est important que les évaluateurs passent suffisamment de temps à réviser et soient sûrs que leur «LGTM» signifie «ce code est conforme à nos normes ». Cependant, les réponses individuelles devraient idéalement être rapides de toute façon .

Examen du code entre les fuseaux horaires

Lorsque vous travaillez entre différents fuseaux horaires, essayez de répondre à l'auteur pendant qu'il est encore au bureau. S'il est déjà rentré chez lui, assurez-vous d'essayer d'envoyer une réponse avant de revenir au bureau le lendemain.

LGTM réservé

Pour des raisons de rapidité, il existe certaines situations dans lesquelles le réviseur doit donner LGTM / approbation, même dans le cas de commentaires sans réponse sur le CL. Cela se fait si:

• L'évaluateur est convaincu que le développeur prendra correctement en compte tous les commentaires restants.
  
• Les autres modifications sont mineures et facultatives .

Le réviseur doit indiquer laquelle de ces options il veut dire si ce n'est pas clair.

Les LGTM réservés sont particulièrement utiles lorsque le développeur et le réviseur sont dans des fuseaux horaires différents, sinon le développeur attendra toute la journée pour obtenir «LGTM approuvé».

Large CL

Si quelqu'un vous envoie un code de révision si volumineux que vous ne savez pas quand vous pouvez l'afficher, alors la réponse typique serait de demander au développeur de diviser le CL en plusieurs CL plus petits . Ceci est généralement possible et très utile pour les réviseurs, même si cela nécessite un travail supplémentaire de la part du développeur.

Si le CL ne peut pas être divisé en CL plus petits et que vous n'avez pas le temps de parcourir rapidement tout cela, écrivez au moins quelques commentaires sur la conception globale du CL et renvoyez-les au développeur pour amélioration. L'un de vos objectifs en tant que réviseur doit toujours être de «déverrouiller» le développeur ou de lui permettre de prendre rapidement toute autre mesure sans sacrifier la qualité du code.

Améliorations de la révision du code au fil du temps

Si vous suivez ces recommandations et approchez strictement la révision du code, vous constaterez que l'ensemble du processus s'accélère et s'accélère au fil du temps. Les développeurs découvriront ce qui est requis pour un code de haute qualité et vous enverront des CL excellents dès le début, nécessitant de moins en moins de temps pour les visualiser. Les examinateurs apprennent à répondre rapidement et à ne pas ajouter de retards inutiles au processus d'examen. Mais ne compromettez pas les normes d'examen de code ou la qualité pour une amélioration imaginaire de la vitesse - en fait, vous n'obtiendrez pas d'accélération globale à long terme.

Situations d'urgence

Il existe également des situations d'urgence où les CL doivent passer très rapidement tout le processus de révision du code et où les principes de qualité devront être assouplis. Mais veuillez lire la description des situations qui peuvent être qualifiées d'urgence et celles qui ne le sont pas.

Comment écrire des commentaires dans la révision de code

Résumé

• Soyez poli.
  
• Expliquez votre raisonnement.
  
• Évitez les commandes explicites en signalant simplement les problèmes et en laissant le développeur décider.
  
• Encouragez les développeurs à simplifier le code ou à ajouter des commentaires au lieu d'explications simples de la complexité.

Politesse

En général, il est important d'être poli et respectueux, et également très clair et utile au développeur dont vous consultez le code. Pour ce faire, vous pouvez vous assurer de toujours faire des commentaires sur le code et jamais sur le développeur . Vous n'avez pas toujours à suivre cette pratique, mais vous devez l'utiliser lorsque vous dites quelque chose de désagréable ou de controversé. Par exemple:

Mauvais: "Pourquoi avez - vous utilisé des flux ici s'il est évident que la concurrence n'est d'aucun avantage?"

Bon: «Le modèle de concurrence ici ajoute de la complexité au système, et je ne vois aucun avantage réel en termes de performances. Puisqu'il n'y a aucun avantage en termes de performances, il est préférable que ce code soit monothread plutôt que d'utiliser plusieurs threads. »

Expliquez les raisons

Dans le «bon» exemple ci-dessus, vous pouvez voir que cela aide le développeur à comprendre pourquoi vous faites votre commentaire. Vous n'avez pas toujours besoin d'inclure ces informations dans vos commentaires, mais il est parfois approprié de donner un peu plus d'explications sur votre logique, les meilleures pratiques que vous suivez ou comment votre suggestion améliore la qualité du code.

Directions

En général, faire des corrections est la tâche du développeur, pas du réviseur. Vous n'êtes pas obligé de développer un projet de solution détaillé ou d'écrire du code pour le développeur.

Cependant, cela ne signifie pas que le critique ne peut pas aider ici. En général, un équilibre approprié devrait être trouvé entre l'identification des problèmes et la fourniture d'une assistance directe. Souligner les problèmes et donner au développeur la possibilité de prendre une décision aide souvent le développeur à apprendre et facilite la révision du code. Cela peut également conduire à une meilleure solution, car le développeur est plus proche du code que le réviseur.

Cependant, des instructions directes, des suggestions ou même du code sont parfois plus utiles. Le but principal d'une révision de code est d'obtenir le meilleur CL possible. L'objectif secondaire est d'améliorer les compétences des développeurs afin que l'examen prenne de moins en moins de temps pour eux.

Acceptez l'explication

Si vous demandez au développeur d'expliquer un morceau de code incompréhensible, cela conduira généralement au fait qu'il le réécrira plus clairement . Parfois, l'ajout d'un commentaire est également une réponse appropriée, s'il ne s'agit pas simplement d'une explication d'un code trop complexe.

Les explications écrites uniquement dans l'outil de révision n'aideront pas les futurs lecteurs. Ils ne sont acceptables que dans certains cas, par exemple, lorsque vous regardez une zone que vous ne connaissez pas très bien, et le développeur explique ce que les lecteurs de code ordinaires devraient déjà savoir.

Comment surmonter la résistance dans le processus de révision du code

Parfois, un développeur se dispute dans un processus de révision de code. Soit il n'est pas d'accord avec votre proposition, soit il se plaint que vous êtes trop strict en général.

Qui a raison?

Lorsque le développeur n'est pas d'accord avec votre proposition, prenez d'abord le temps de réfléchir à sa position. Souvent, il est plus proche de votre code et peut donc vraiment avoir une meilleure idée de certains aspects. Est-ce que cela a du sens dans son argument? Cela a-t-il un sens en termes de qualité du code? Si c'est le cas, admettez qu'il a raison et la question disparaîtra.

Cependant, les développeurs n'ont pas toujours raison. Dans ce cas, le réviseur doit expliquer davantage pourquoi il pense que sa proposition est correcte. Une bonne explication montre à la fois une compréhension de la réponse du développeur et des informations supplémentaires sur les raisons pour lesquelles le changement est nécessaire.

En particulier, lorsque le réviseur estime que sa proposition améliorera la qualité du code, il devrait insister sur celui-ci s'il considère que l'amélioration de la qualité qui en résulte justifie le travail supplémentaire. L'amélioration de la qualité du code, en règle générale, se produit par petites étapes.

Parfois, il faut plusieurs séries d'explications avant de l'accepter. Assurez-vous simplement que vous restez toujours poli , et faites savoir au développeur que vous l'entendez, mais ne soyez pas d'accord.

À propos du ressentiment des développeurs

Les évaluateurs estiment parfois qu'un développeur est contrarié si un évaluateur insiste sur l'amélioration. Parfois, les développeurs sont vraiment contrariés, mais généralement pas pour longtemps, et plus tard, ils vous seront très reconnaissants de les avoir aidés à améliorer la qualité de leur code. Habituellement, si vous êtes poli dans vos commentaires, les développeurs ne sont pas vraiment du tout contrariés, et la préoccupation n'est que dans la tête du critique. Habituellement, le style de rédaction des commentaires est plus frustrant que la persistance du réviseur quant à la qualité du code.

Modifications en attente

Une source typique de controverse est que les développeurs (pour des raisons évidentes) veulent faire le travail. Ils ne veulent pas passer par une autre série d'examens pour ce CL. Par conséquent, ils disent qu'ils supprimeront quelque chose dans le dernier CL, et maintenant vous devez créer LGTM pour ce CL. Certains développeurs le font très bien et écrivent immédiatement un autre CL qui résoudra le problème. Cependant, l'expérience a montré que plus il s'écoule de temps après le CL d'origine, moins il est probable que cette modification se produise. En fait, généralement si le développeur n'effectue pas l'édition immédiatementqui ne le fait généralement jamais. Non pas parce que les développeurs sont irresponsables, mais parce qu'ils ont beaucoup de travail et que l'édition est perdue ou oubliée sous la rampe d'autres travaux. Ainsi, il est généralement préférable d'insister sur un correctif immédiat avant que la validation n'entre dans la base de code et ne soit «terminée». Autoriser les modifications en attente est un moyen typique de dégénérer les bases de code.

Si le CL introduit une nouvelle complexité, alors il doit être corrigé avant l'envoi, si ce n'est pas une urgence. Si CL ​​montre d'autres problèmes qui ne peuvent pas être résolus pour le moment, le développeur doit enregistrer un bogue dans le tracker et l'assigner à lui-même afin qu'il ne se perde pas. Il peut éventuellement écrire un commentaire TODO dans le code concernant cette erreur.

Plaintes générales de gravité

Si vous aviez l'habitude d'effectuer une révision de code plutôt superficielle et de passer en mode strict, certains développeurs commenceront à se plaindre très fort. L'augmentation de la vitesse de révision du code résout généralement ces problèmes.

Parfois, il peut s'écouler des mois avant que les plaintes ne disparaissent, mais en fin de compte, les développeurs ont tendance à voir la valeur des révisions de code strictes lorsqu'ils voient à quel point le code est bon. Parfois, les manifestants les plus bruyants deviennent même vos partisans les plus forts quand quelque chose se produit qui leur fait vraiment voir la valeur de critiques rigoureuses.

Résolution des conflits

Si vous effectuez toutes les actions ci-dessus mais rencontrez toujours des conflits, reportez-vous à la section Code Review Standard pour obtenir des recommandations et des principes qui peuvent aider à résoudre le conflit.