Revue de code: mauvais conseils pour le contributeur et le réviseur

Salut Je m'appelle Nikolai Izhikov. Dans cet article, je veux parler d'un élément d'interaction important que nous rencontrons dans le processus de développement de logiciels, en particulier en open source. Il s'agit d'une procédure pas à pas et d'une révision de code.


Je donnerai des conseils néfastes sur la façon de créer ma fonctionnalité et de la fusionner dans l'assistant de projet open source dans le contexte des différences culturelles, temporelles, conceptuelles et autres entre les membres de la communauté. C'est généralement une question délicate, surtout pour ceux qui débutent en open source.

Commençons par une petite introduction. Lorsque vous communiquez avec des personnes, en particulier par chat ou par courrier, en particulier en anglais, gardez toujours à l'esprit que votre message peut être perçu complètement différemment de ce que vous attendiez. On ne sait pas qui lira la lettre. Il peut être hindou, anglais ou simplement somnolent. Il y aura toujours des différences dans la compréhension de mots spécifiques, et sans entrer dans les détails techniques, je vais vous dire comment les minimiser.

Attention: l'histoire contient du sarcasme.



Mauvais conseils pour un contributeur


Ne discutez d'une nouvelle fonctionnalité avec personne


Si vous souhaitez implémenter une nouvelle fonctionnalité, ne discutez en aucun cas de votre révision avec qui que ce soit. Quelqu'un peut écouter et le faire avant vous! Ou, après avoir entendu les détails, ils peuvent vous dire que vous n'avez pas compris quelque chose - pour critiquer votre idée. La critique fait très mal. Peut-être qu'une fonctionnalité comme la vôtre a déjà été discutée, et vous en avez besoin d'une autre ... En général, en aucun cas, ne dites à personne ce que vous voulez faire. Jamais.



Ne faites jamais de documentation technique


Les passionnés et les gars expérimentés de la communauté aiment beaucoup comprendre le code du patch reçu. Par conséquent, en aucun cas, ne faites pas de documentation technique. Wiki, Jira, discussion sur la liste de diffusion - tout cela est complètement absurde. Et les conneries ne sont pas pour toi!

Lorsque vous envoyez un patch à [+5000, -3500] avec une description des améliorations sous la forme d '«améliorations des méthodes d'usine et d'autres améliorations», tout le monde le comprendra tout en comprenant à quel point vous êtes bon.



Ne jamais exécuter de tests


Tous les tests sont dangereux car ils peuvent casser quelque chose. Si vous avez quand même réussi à exécuter les tests, n'affichez pas leurs résultats. Des erreurs dans les résultats peuvent entraîner le rejet du patch! Et certainement jamais écrire de nouveaux tests. Tout exécuter sera plus long, la consommation du processeur augmentera. Quoi qu'il en soit, les bons développeurs écrivent du code sans bogues - tout collègue expérimenté examinera vos correctifs et comprendra qu'il n'y a aucune erreur.



Ne jamais lire le guide pratique


Dans un projet open source, ne lisez jamais aucun How-To. Ils sont écrits pour des imbéciles, non?
Concevez votre code à votre manière. Sinon, comment tout le monde comprendra que vous êtes un bon développeur et une personne créative polyvalente avec un sens développé de la beauté?

Envoyez des correctifs à votre convenance. Par exemple, le projet est lié à l'infrastructure GitHub - envoyez-le tel qu'il est envoyé au noyau Linux, juste dans la lettre. Vous pouvez même pas dans la pièce jointe, mais directement dans le texte. Après tout, Linus Torvalds ne conseillera pas mal! Et si le projet est adopté différemment, alors ce sont des problèmes du projet.



N'hésitez pas à compliquer l'API publique


Lors du développement de la nouvelle API publique, essayez de la rendre aussi abstraite et complexe que possible. Il est toujours possible de répondre à toutes les questions des utilisateurs sur le comportement non évident de l'API: «N'avez-vous pas lu le manuel? Page 42, tout est écrit clairement! Relisez-le! ” Dans les projets sérieux, tout devrait être compliqué. Nous avons un projet sérieux?



Ne conseille pas ou ne parle pas des problèmes


Ne consultez personne et ne parlez à personne des problèmes rencontrés lors du développement. C’est beaucoup plus amusant de comprendre une chose. De toute façon, les bons développeurs réussissent toujours la première fois, ils n'ont aucun problème. Si d'autres découvrent vos problèmes, ils deviendront plus intelligents et écriront du code plus rapidement que vous. Cela ne devrait pas être autorisé. Et en effet, il n’est pas habituel de parler de ses problèmes.

Les limites de la décision finale ne valent pas non plus la peine d'être mentionnées. Après tout, la solution peut être demandée de finaliser. Qui se soucie des restrictions? Lorsqu'un utilisateur commence à introduire votre produit dans le produit et rencontre des restrictions, il sera plus intéressé et plus amusant. Il viendra vers vous et vous demandera de le terminer. Et jusqu'à ce point, en aucun cas ne lui parlez des restrictions - et s'il décide de ne rien mettre en œuvre?

Un très bon critique trouvera tout et il vous demandera des détails. En aucun cas, ne lui dites rien.



Si vous avez des questions, écrivez à la liste des développeurs


Cette astuce complète la précédente. Il est préférable non seulement de ne rien dire à personne, mais aussi de poser des questions principalement sur la liste des développeurs.

Voici des exemples de questions auxquelles tout le monde aime répondre. Si vous écrivez une sorte de test, assurez-vous de demander: "Avez-vous besoin de vérifier la valeur null de cette collection?". Vous n'avez pas besoin de le découvrir par vous-même, vous pouvez toujours demander sur la feuille de développement. Après tout, il y a beaucoup de gens qui n'attendent que de se poser une telle question. Ils chercheront à y répondre plus rapidement.

"Comment faire la tâche?" - Une autre grande question. En aucun cas, n'indiquez aucun détail: l'ID de tâche sera suffisant. Tous ceux qui en ont besoin verront par eux-mêmes.

"Quel cadre utiliser?" - également une très bonne question. Il est toujours intéressant d'y répondre et vous pouvez débattre.



Résoudre tous les problèmes de projet en une seule demande de tirage


Ceci est mon conseil préféré. Résolvez tous les problèmes du projet en une seule demande, surtout si vous travaillez en entreprise. Il n'y avait que des imbéciles dans le projet avant vous, ils ont mal écrit le code. Et tu écris bien. En conséquence, vous devez simplement:

  • Refactorisez tout le code existant que vous ne comprenez pas;
  • renommer toutes, à votre avis, les variables mal nommées;
  • corriger tous les commentaires javadoc existants.

En général, vous pouvez prendre et retirer certaines classes, usines, etc., effectuer d'autres transformations pour que le code soit meilleur. Cela semblera particulièrement impressionnant dans les domaines qui ne sont pas pertinents pour votre tâche. Ainsi, vous pouvez révéler plus pleinement votre potentiel. À la gloire de la POO! Amen!



Demander une révision du code à l'avance


Lorsque vous effectuez une tâche, puis envoyez une demande de révision de code, le processus peut prendre un certain temps. Tous les experts sont généralement occupés. Profitez de l'astuce: lorsque votre tâche, comme il vous semble, se terminera bientôt, demandez une révision à l'avance. Après tout, ils ne regarderont pas tout de suite - jusqu'à ce que vos mains vous atteignent, vous pouvez tout finir.

Eh bien, si le réviseur a soudainement le temps, alors que la tâche n'est pas encore terminée, il n'a pas eu de chance. Expliquez la situation, dites que demain tout sera prêt. De cette façon, vous accélérez le processus (au moins par révision) et accélérez la fusion.



Fatigué de la tâche - laissez tomber


Tous ceux qui travaillent en open source ont beaucoup de temps. Pas besoin de finir quoi que ce soit. Passé l'examen du code, reçu des commentaires. Et pourquoi éditer et apporter quelque chose à fusionner? Prenez le casse-tête suivant et faites-le. C'est la voie du succès! L'évaluateur a beaucoup de temps et le prochain patch sera sans résultat!



Le critique est votre ennemi


Et quoi d'autre pour nommer la personne qui se tient entre vous et le commit en maître? La critique du code est une critique de vous personnellement! Qui pense-t-il être? En communication personnelle, «bombardez» autant que possible! Sinon, comment l'examinateur sait-il que vous vous souciez? C'est la règle de base du développement!

Je recommande ce conseil dans le développement quotidien. Cela aide à atteindre le résultat et à le faire correctement.



Mauvais conseils pour le critique


N'automatisez pas les contrôles de routine


Si vous avez atteint le niveau dans le projet alors que des correctifs pour révision vous sont déjà envoyés, n'automatisez en aucun cas les contrôles de routine! Il est beaucoup plus amusant de passer quelques cycles d'examen sur le dépannage et la discussion:

  • formatage du code;
  • dénomination des variables;
  • vérifie si ces variables sont marquées finales, ce qui peut être marqué par elles;
  • ... et tout le reste.

En aucun cas, les contrôles de routine ne doivent être automatisés. Oui, et des tests pour conduire à rien.



Ne révélez jamais toutes les règles du jeu jusqu'à la toute fin.


Pour être en avance, vous devez toujours garder une paire d'as dans votre manche. Ne parlez à personne de la nécessité d'une compatibilité descendante. Il vaut mieux dire juste avant le commit: «Ici, selon nos règles, la compatibilité descendante doit être assurée. Veuillez corriger. " Cela sera particulièrement efficace si vous avez déjà examiné cinq fois. Le sixième, on peut toujours dire: "Je ne suis pas un expert, vous devez donc attendre l'examen de M. X, qui regardera à nouveau."

De tels commentaires, en particulier au stade final de l'examen, ajoutent toujours de la motivation aux contributeurs.



Émotions, autorités et non merci


Cette astuce chevauche la dernière astuce des contributeurs. Rédigez des commentaires sur la demande de tirage aussi émotionnellement que possible. Si quelque part n'est pas vérifié pour null ou si la variable n'est pas nommée de cette façon, ajoutez de la passion à vos commentaires. Faites-leur voir que vous vous souciez.

En cas de litige, ne donnez en aucun cas d'arguments techniques. Avec des arguments techniques, il se peut que vous vous trompiez. Référez-vous aux autorités - c'est le meilleur argument du différend, vous gagnerez toujours.

Si quelqu'un a passé en revue votre avis, vous ne devriez jamais dire merci. Ils veulent toujours s'engager dans l'open source! En aucun cas, je ne dois remercier, et ils reviendront donc!



Maintenant, sérieusement: à quoi faut-il penser lors de la préparation et de la réalisation d'une révision de code?


J'espère que tout le monde a compris comment mener et réussir l'examen? Dans chacun de ces conseils, en plus du sarcasme, il y a une douleur qui se produit souvent dans la pratique.



Je serai le capitaine d'Evidence et je vous dirai à quoi vous devez vraiment penser lors de la préparation et de la conduite d'une révision du code. Des considérations s'appliquent en outre à la fois à celle qui développe la fonctionnalité et à celle qui la révisera. Pourtant, ce sont les deux faces d'une même pièce.
Le consensus dans la communication est, d'une part, réalisable, et d'autre part, il est nécessaire que le projet aille de l'avant. Actuellement, peu de produits peuvent être développés seuls. Il s'agit généralement d'un travail d'équipe.

Faites preuve de bon sens


C'est le conseil le plus important. Faites preuve de bon sens, cela oriente. Il me semble que ce conseil s'applique à toutes les situations de la vie. Si vous faites quelque chose, demandez-vous si cela répond à la simple règle de bon sens?

Supposons ...


Il s'agit de culture. Je suis allé dans plusieurs grandes communautés open source. Je ne sais pas si cela fait partie de la mentalité russe, mais souvent une personne qui peut être perçue comme un patron est simultanément perçue comme une personne qui s'est accidentellement mise en place. On pense qu'il vous veut du mal, par défaut il y a des doutes sur son professionnalisme. Par conséquent, il est très important dans tout travail de supposer au moins une seconde que:

  • Le réviseur (contributeur, votre patron ou collègue) n'est pas votre ennemi . Il ne veut pas que tu sois mauvais. C'est une hypothèse simple, mais souvent elle n'est pas faite. Je lui conseille de faire tout de même.
  • La personne qui vous écrit des commentaires est également un bon ingénieur. Vous, bien sûr, êtes bon, avez fait une telle fonctionnalité. Mais il existe de nombreux autres bons ingénieurs dans le monde. Et celui qui vous a envoyé des commentaires s'applique également à eux.
  • Cette personne souhaite également que votre tâche soit accomplie.

    Quand il demande quelque chose, il a une sorte de motivation. Il le fait pour une raison. Surtout si cette personne n'est pas le premier jour du projet. Il y a sûrement une raison. Vous pouvez poser des questions sur cette raison: pourquoi avez-vous besoin de faire exactement cela? Pourquoi la compatibilité descendante est-elle nécessaire ici? Si vous posez des questions simples calmement et raisonnablement et écoutez les réponses, vous pouvez en faire plus.

Quelle valeur apportera le produit?


La revue n'est pas seulement des correctifs prêts à l'emploi, mais aussi des améliorations de projet, des corrections. En fait, la révision du code commence au moment où vous venez de discuter de votre révision. À ce stade, demandez-vous: quelle valeur le produit apportera-t-il au produit?

  • Sera-ce une nouvelle fonctionnalité?
  • S'agit-il d'une amélioration existante?
  • Extension d'une fonctionnalité existante?
  • Sera-ce du refactoring de code? Il n'y a rien de mal à cela. Certains critiquent la refactorisation, mais c'est nécessaire. Et vous devez savoir que vous le faites, et non une nouvelle fonctionnalité ou autre chose.
  • Accélère-t-il un processus, améliore-t-il les performances?
  • Est-ce une correction de bogue?

Il existe d'autres options. Dans tous les cas, en commençant à développer quelque chose, à résoudre un problème, vous devez comprendre quelle valeur vous ajouterez au projet.

Pourquoi la révision (fonctionnalité) est-elle comme ça?


Il y a un certain nombre de questions utiles à poser.

Pourquoi créer une fonctionnalité? Pourquoi cette fonctionnalité est-elle nécessaire? La réponse à cette question est importante.

Où est le début des travaux? Dans ma pratique, il m'est arrivé de me demander de refaire une certaine application. Il y a une application A, vous devez faire une application B, qui fait presque la même chose avec des changements mineurs. Je commence à faire le travail et il s'avère que A ne fonctionne pas. En fait, il est utilisé quelque part dans la production en utilisant l'interface homme-machine - c'est-à-dire que quelqu'un s'assoit et redémarre constamment le programme, fixant littéralement l'exception du pointeur nul. Où est le début des travaux? Dans ce cas, il s'agira de fixer le programme A pour qu'il fonctionne de manière stable, puis d'écrire le programme B.

Où est l'achèvement complet des travaux? Comment le travail effectué devrait-il être idéal? Il est important de formuler dès le début où vous allez.

Où est la fin de l'étape actuelle? Il est clair que vous ne pouvez pas manger un éléphant tout de suite et il vaut mieux décomposer le travail en étapes. Il est important de comprendre où se trouve la fin de l'étape actuelle. Souvent, les projets sont gonflés et ne se terminent pas simplement parce que la portée du projet devient infiniment grande.

Pourquoi la fonctionnalité est-elle décomposée précisément à ces étapes? Il s'agit de MVP et tout ça. Pensez-y aussi.

Maintenant sur l'API publique


Il existe de nombreux articles sur les propriétés de l'API publique. Lisez-le avant de l'implémenter. Comme bon exemple, je peux citer le framework JQuery, Spring en Java. Il existe également des exemples négatifs. Ceux qui ont programmé en Java pendant des années se souviennent probablement du terrible du point de vue de l'API publique EJB 2.1. La fonctionnalité peut être bonne, mais si l'API publique est mauvaise, personne ne peut convaincre les utilisateurs d'utiliser le produit.

L'API publique n'est pas seulement un outil pour les utilisateurs tiers. Ceci et l'API des composants internes, que vous réutilisez vous-même entre eux. Propriétés importantes de l'API publique:

  • Simplicité.
  • Les preuves.
  • Les valeurs par défaut correctes. Il vaut la peine d'y penser, si, par exemple, dans un environnement de test, vous créez 500 threads, comme en production. Ou vice versa, en production par défaut il y a 3 threads, comme dans un environnement de test.
  • Effacer les messages d'erreur. C'est le fléau de tant de produits. Quand quelque chose tombe à l'intérieur du système, il n'est pas clair ce qui est mal fait. Hier travaillé, aujourd'hui une exception de pointeur nul. Ce que vous avez fait de mal et comment y remédier n'est pas clair dans le message d'erreur.
  • Il est difficile de se tromper. Il y a beaucoup de recommandations à ce sujet. Les contrôles de temps de compilation sont toujours meilleurs que les contrôles d'exécution, etc.
  • Journaux clairs et suffisants. Ceci est particulièrement important lorsque vous écrivez du code qui sera réutilisé et déployé quelque part sur le serveur.
  • La capacité de surveiller. Vous devez comprendre que les journaux et la surveillance font également partie de votre API publique. Lors de l'analyse des erreurs, les utilisateurs verront comment les mesures que vous crachez dans la surveillance se comportent.

Modifications du sous-système


Lorsque vous arrivez à la révision du code, il est important d'avoir en tête une liste complète des systèmes et sous-systèmes d'un grand produit que vous modifiez. Dans les projets d'entreprise, cela peut ne pas être clair: qu'il s'agisse d'un schéma de base de données, ou d'un contrôleur, ou d'une présentation, d'une sorte de système de rapport, de téléchargements, de téléchargements, etc.

Lorsque vous travaillez avec un produit en boîte, il est important de vous poser la question: comment les changements affectent-ils les processus existants dans le système? Existe-t-il une rétrocompatibilité? Cela affecte-t-il les performances? Si cela affecte, alors la performance de quoi? Comment cela affecte-t-il l'expérience utilisateur?

Processus système standard


Chaque système d'entreprise a des processus standard: démarrage, installation, une liste d'opérations. Comment vont-ils couler maintenant? Avant la révision du code, il est important de comprendre cela. Vous devez parcourir le code qui implémente ces processus. Dans le cas d'Ignite, c'est:

  • Démarrage / arrêt du nœud (serveur / client, coordinateur / régulier) - démarrer, arrêter un nœud, démarrer un serveur ou un nœud client, démarrer un coordinateur ou un nœud normal
  • Node Join - en termes de nouveau nœud / coordinateur / serveur / client
  • Activation (et désactivation) du cluster
  • Changement de coordinateur
  • Créer / supprimer le cache
  • Persistance / BLT / MVCC

Il est clair que l'ensemble de ces processus est assez large. Il est important de comprendre que de tels processus existent et comment ils changent.

Étuis d'angle


Dans votre application métier, la reprise après sinistre, l'initialisation initiale du système, l'arrêt du nœud, le redémarrage, la mise à jour de votre application, etc. peuvent se produire. Dans le cas d'Ignite, nous avons les cas d'angle suivants:

  • changement de coordinateur;
  • chute de nœud;
  • problèmes de réseau - lorsque les messages réseau n'atteignent pas;
  • fichiers cassés.

Nous devons vérifier et vérifier ces choses que tout est en ordre.

Bonnes fonctionnalités de code


Nous sommes donc arrivés au code. Je vous conseille de ne pas être paresseux et de chercher en elle:

  • simplicité
  • extensibilité
  • testabilité
  • la fiabilité
  • grande vitesse de travail.

Accès simultané


Java a ses propres particularités lors de l'écriture de code de concurrence. Si vous avez un système d'entreprise et qu'il y a peu de concurrence, vous n'avez pas besoin de prendre en compte ces fonctionnalités. Cependant, une synchronisation passe généralement par la base de données. Dans des choses comme Ignite, c'est un peu plus compliqué. Et ici, non seulement la fonctionnalité du code est importante, mais aussi ses propriétés:

  • Est-il difficile de comprendre votre modèle de simultanéité?
  • Structure des données partagées - comment leur intégrité est-elle garantie?
  • Serrures - quoi et pourquoi?
  • Threads - quels pools sont utilisés? Pourquoi?
  • Garanties de visibilité des changements - à quoi servent-ils?

Ces questions doivent être posées avant de réviser le code de concurrence. Il est clair que cette liste peut être prolongée très longtemps.

Tests de performance. Repères


Si vous développez une sorte de système, il a des clients, des installations, alors cela fonctionne évidemment avec des performances. Dans le monde d'aujourd'hui, il est impossible d'augmenter la puissance matérielle indéfiniment. Nous avons besoin de tests et d'une surveillance des performances. Lors de la révision d'un code, il est important de comprendre:

  • Les tests de performances sont-ils nécessaires? C'est peut-être une sorte de raffinement qui n'a pas besoin de tests de performances?
  • Si nécessaire, lequel? Il existe de nombreuses techniques et méthodes de mesure, etc.
  • Où-comment-que faut-il mesurer?
  • Quels repères sont indicatifs? Le nombre de nœuds? Du fer? Allumer la configuration? La nature de la charge?

Total


Dans l'ensemble, la révision du code est une pratique très enrichissante. J'espère que tous les développeurs (y compris les produits d'entreprise) l'ont déjà implémenté à la maison. Sinon, veuillez l'implémenter dès que possible. Je serai heureux de discuter des pratiques de révision de code avec vous dans les commentaires.

Vidéo de la conférence:

La présentation est disponible ici .

Source: https://habr.com/ru/post/fr445440/


All Articles