Je m'engage constamment sur des projets open source (Red Hat, etc.). Et il a remarqué que les revues de code négatives, qui sont essentiellement subjectives, prennent le plus de temps. Le plus souvent, cela se produit avec des validations, où le responsable pour une raison quelconque n'aime pas votre changement. Dans le meilleur des cas, une telle stratégie de révision de code se traduit par une perte de temps dans des débats dénués de sens; dans le pire des cas, cela décourage activement l'engagement, créant un environnement hostile et élitiste.

La révision du code doit être objective, concise et, si possible, ne contenir que certains faits. Ce n'est pas un débat politique ou émotionnel, mais technique. Son objectif est d'avancer, de développer le projet et tous les participants. Tout engagement ne doit être évalué que sur le fond et non sur une opinion subjective.

Stratégies de révision du code

Voici quelques stratégies à garder à l'esprit lors de l'examen du code que vous (en tant que mainteneur) n'aimez pas pour une raison quelconque:

1. Reformuler l'objection comme une question

• Mauvais: "Ce changement rendra XXX impossible" (C'est une exagération; est-ce vraiment impossible?)
  
• Bon: "Comment allons-nous faire XXX après votre changement?"

2. Évitez d'exagérer

Exprimez simplement vos préoccupations et posez des questions pour obtenir le résultat souhaité.

• Mauvais: "Ce changement détruira la productivité."
  
• Bon: «Il semble que X soit plus lent que le Y existant; "avez-vous pris des mesures / collecté des données pour montrer que ce n'est pas le cas?"
  
• Mieux (si vous avez le temps): «Pour l'instant, je collecte des données. Je vais essayer de vérifier que X n'est pas plus lent que Y. "
  
• Aussi bon: «Ce changement change un cycle O (n) unique en un cycle O (n²) double imbriqué; cela affectera-t-il les performances? "

3. Laissez-vous des commentaires malveillants

Certaines pensées sont mieux gardées avec vous. Si vous ne pouvez pas dire poliment, mieux vaut garder le silence.

• Mauvais: "Je pense que c'est un mauvais changement qui va tout gâcher."
  
• Mauvais: "Êtes-vous sûr que le développement de logiciels est le bon choix de carrière pour vous?"

4. Agissez positivement

Peut-être que vous aviez une idée différente sur la façon de résoudre le problème? Si vous agissez positivement, vous finirez par trouver une solution meilleure que n'importe laquelle des options d'origine.

• Mauvais: "Ce changement est nul, ma version est meilleure."
  
• Bon: "J'ai aussi un changement pour ce lieu: peut-être pouvons-nous comparer et / ou combiner des idées."
  
• C'est aussi bien. "J'ai un changement similaire dans mon travail, mais j'ai décidé de faire X, parce que ZZZ; pourquoi avez-vous choisi Y? "

5. N'oubliez pas que chacun a une expérience différente.

À tous les autres égards, un ingénieur absolument compétent peut ne pas connaître pendant quelques années certains faits que vous considérez comme du bon sens. Il n'y a rien de mal à dire la chose évidente, à moins que vous n'ayez une condescendance ou une malveillance.

• Mauvais: "Vous ne voyez pas que c'est clairement faux?"
  
• Bon: "Ce n'est pas vrai car il lève une exception de pointeur nul lorsque X est Y."

6. Ne minimisez pas la complexité de ce qui n'est pas évident

N'oubliez pas que les choses qui sont évidentes pour vous peuvent ne pas l'être pour tout le monde. En suggérant des approches alternatives et en montrant des exemples utiles, vous pouvez aider tout le monde à se synchroniser.

• Mauvais: "Pourquoi ne pas simplement écraser la gnose?"
  
• Bon: "Si vous écrasez la gnose, alors cette partie peut devenir plus facile (voir XXX pour un exemple)."

7. Respect

Parfois, un commit ne répond tout simplement pas à la norme de qualité minimale. Il est normal de le dire, mais faire preuve de respect ne nécessite aucun effort supplémentaire.

• Mauvais: "C'est un code stupide écrit par une personne stupide."
  
• Bon: «Merci pour votre contribution. Cependant, il ne peut être adopté tel quel; il y a beaucoup de problèmes (comme indiqué ci-dessus). »
  
• Aussi bien: «Comme indiqué ci-dessus, il y a plusieurs problèmes avec ce commit. Peut-être prendre du recul et parler de scénarios d'utilisation? Cela aidera à trouver le chemin. "

8. Gérez vos attentes (et votre temps)

Si le commit est trop important et ne peut pas être rapidement considéré, il est normal de le dire tout de suite. Cherchez ensuite une solution.

• Mauvais: "Je ne l'accepte pas, c'est trop grand."
  
• Aussi mauvais: ignorez le commit jusqu'à ce qu'il disparaisse.
  
• Bon: «Pourriez-vous le diviser en commits plus petits? Je n'ai pas trop de temps pour un examen du code, mais c'est juste un engagement trop important / compliqué pour un examen. "

9. Dites «s'il vous plaît» (montrez la courtoisie)

En disant simplement «s'il vous plaît», vous montrez que vous respectez l'heure de l'expéditeur, en particulier lorsque vous souhaitez modifier la mise en forme ou le style, ce qui peut sembler être un léger changement. Exemples:

• "Pourriez-vous documenter les changements dans les lacunes d'une autre demande de pool?"
  
• "Pourriez-vous aligner ces définitions de variables pour les rendre plus faciles à lire?"

10. Démarrer une conversation

Si après tout cela, vous n’aimez toujours pas quelque chose, mais vous ne savez pas pourquoi, vous devrez peut-être simplement le supporter. Ou dites: "Je n'aime pas ça, mais je ne sais pas pourquoi, pouvons-nous en parler?" C'est une question raisonnable, et même si cela peut prendre un peu de temps, cela en vaut souvent la peine car maintenant vous avez deux personnes, et les deux apprennent (expliquent et écoutent), et non pas deux personnes qui s'opposent.

Même des ingénieurs qualifiés et expérimentés devraient pouvoir dire: «Je ne comprends pas pourquoi je n'aime pas ça.» Ce n'est pas une invitation à attaquer la position de l'examinateur, mais plutôt une recherche honnête de connaissances.

Résumé

Évitez les déclarations hyperboliques ou pompeuses, évitez les disputes, les expressions élitistes ou dégradantes et les constructions comme "évidentes" et "pourquoi ne pas simplement ...". Utilisez des déclarations claires et factuelles et un langage à l'appui, posez des questions et allez de l'avant. N'oubliez pas que les collègues et les contributeurs sont également des personnes. Leur temps mérite le même respect que le vôtre.