Je suis tombé sur un document contenant des règles et des recommandations sur le processus de révision du code au sein de l'entreprise. J'ai décidé que ces informations utiles devraient être partagées avec le monde extérieur. Avec la bénédiction de l'
auteur, je publie l'Ĺ“uvre.
La plupart de ces règles sont de nature consultative et doivent être guidées principalement par le bon sens, et non par une observation aveugle.
Dispositions générales
- Dans hh.ru, l'examen a lieu dans le format de demande de pull sur Github.
- Les demandes de révision et d'extraction sont requises, même si, dans le cadre de la tâche, une ligne ou un caractère du code est modifié.
- Chaque tâche doit avoir au moins un réviseur responsable, elle est indiquée dans la tâche en tant que réviseur et est responsable du code, comme l'auteur. Non interdit et encouragé à participer à l'examen d'autres personnes.
- La responsabilité de la réalisation de la tâche par le biais de la revue incombe à l'auteur de la tâche.
- Toutes les modifications après un examen, telles que les révisions pendant les tests, doivent également être prévisualisées.
Examiner les objectifs
- Diffusion d'expérience et de connaissances entre développeurs.
- Prise en charge du principe - les modifications ne doivent pas aggraver le code existant, elles doivent l'améliorer.
- Correction des erreurs de logique et des erreurs liées à la sécurité, traitement correct des exceptions.
- Amélioration de la qualité du code, de la maintenabilité et de l'architecture du projet.
- Amélioration de la lisibilité du code.
- Amélioration de la couverture des tests et des tests au bon niveau.
- Amélioration de la documentation.
- Conformité des changements avec les exigences de la tâche.
- Prévention de la dette technique ou de la taxe technique.
- La conception réfléchie de l'API, où l'API est n'importe quel module avec une interface externe. Par exemple, qu'une ressource dans un service possède une URL bien pensée, un format JSON pratique, des codes de réponse corrects dans différents cas, etc.
- L'historique correct des validations dans la Gita, avec des messages qui reflètent l'essence du message, sans validations temporaires.
Préparation à l'examen
- Avant de publier une pull request, lisez votre code 2-3 fois: avec une forte probabilité, vous y trouverez vous-même des erreurs, ce qui fera gagner du temps au réviseur. Vérifiez qu'aucune erreur ne peut être détectée automatiquement - erreurs de style de code, tests abandonnés. Assurez-vous que le code n'a pas de commentaires temporaires et de code de débogage, et vérifiez également que votre code correspond aux guides de style acceptés.
- Si vous créez une demande d'extraction en cours de travail sur une tâche, écrivez «[WIP]» dans l'en-tête et mettez une marque «work in progress». Lorsque le travail est terminé, supprimez les balises et informez-en dans un commentaire séparé afin que les parties intéressées sachent que le code peut être consulté.
- Soyez prêt à montrer à l'examinateur une mini-démonstration de la tâche.
- Donnez de petites portions de code pour examen. 200-300 lignes de changement - la limite pour un examen efficace.
- Apportez des modifications indépendantes dans des validations distinctes.
- DĂ©crivez les commits avec des messages courts et informatifs.
- Faites une refactorisation en tant que commit séparé, il est plus facile de voir les changements dans la logique, l'essence de la tâche.
- Formatez le code en tant que validation distincte avant les modifications majeures, ou mĂŞme en tant que demande d'extraction distincte.
- Ne commettez pas de modifications mineures. Par exemple, ajouter des sauts de ligne dans un fichier dans lequel vous n'avez rien changé d'autre.
- Dans le titre de la demande d'extraction, décrivez brièvement et de manière informative l'essence des modifications.
- Décrivez le problème et la solution dans la demande de tirage. Décrivez les solutions alternatives et pourquoi la solution actuelle est sélectionnée. Si les modifications concernent l'interface, joignez les captures d'écran «avant» et «après».
- Dans la demande d'extraction, indiquez un lien vers la tâche et dans la tâche, un lien vers la demande d'extraction.
- Il est parfois utile de discuter de la solution choisie avec le réviseur avant d'écrire le code. Cela vous aidera à trouver la meilleure solution au problème et à simplifier le processus d'examen.
Choix de l'Ă©valuateur
- Confiez la tâche de l'examen à un spécialiste du domaine concerné.
- Si plusieurs commandes fonctionnent avec du code, il est utile de sélectionner un réviseur d'une autre équipe pour diffuser les connaissances.
- L'agent de probation n'est peut-ĂŞtre pas l'examinateur responsable, mais peut participer au processus d'examen.
- Ne confiez pas toutes vos tâches à la revue aux mêmes personnes - demandez des critiques à différentes personnes.
- Si le problème concerne un morceau de code non trivial qu'une certaine personne comprend, montrez-lui les modifications, quel que soit le responsable de l'évaluation. Souvenez-vous également que chaque référentiel a des mainteneurs, ils en savent plus sur ce code et supervisent le développement.
- Le reste du réviseur est choisi arbitrairement, d'un commun accord entre les parties. Utilisez les recommandations sur le github en fonction du «blâme» du code affecté pour vous aider à choisir.
- Après avoir sélectionné un réviseur, spécifiez-le en tant que cessionnaire dans la demande d'extraction. Une liste de toutes les demandes d'extraction qui vous sont attribuées.
Processus d'examen, général
Désigne à la fois l'auteur du code et le réviseur.
- Tout le code est commun, il n'y a pas de concepts tels que «mon code» ou «votre code». Cela signifie que chaque développeur est responsable de n'importe quelle ligne de code, et pas seulement celui qui a écrit cette ligne.
- Créez une atmosphère de compréhension mutuelle, soyez poli et amical, appréciez-vous et respectez-vous les uns les autres, n'imposez pas votre opinion. Écoutez l'opinion de l'adversaire et ne tenez pas votre principe. Ne transformez pas l'examen en une expérience négative pour vos collègues.
- Posez des questions si quelque chose n'est pas clair. Argumentez et spécifiez les questions et réponses. Il peut ne pas être évident pour l'auteur du code ce que l'on entend par la question «pourquoi en est-il ainsi?», Mais le critique ne comprend pas l'argumentation de la réponse «en désaccord».
- N'étirez pas le processus d'examen, gagnez du temps, répondez rapidement aux commentaires, discutez verbalement, ne provoquez pas de longues discussions et n'engendrez pas de «holivars». Si vous ne parvenez pas rapidement à un consensus, demandez l'aide de vos collègues, du responsable ou du responsable du domaine fonctionnel.
- Si la tâche ne se déroule pas sur quelques lignes, passez 10 à 15 minutes avec le réviseur sur une révision conjointe du code, il est utile de le faire avant de créer une demande de tirage. Discutez de l'essence de la tâche et de la solution, voyez comment cela s'est passé et est devenu dans un environnement de travail. Cela aidera le réviseur à mieux plonger dans le contexte de la tâche. La plupart des commentaires resteront non écrits, ce qui fera gagner du temps à tout le monde.
- Après une discussion orale, décrivez toujours la décision prise et les raisons de celle-ci dans la demande de retrait. La responsabilité de la description incombe à l'auteur du code.
- Si le code contient le même type d'erreurs, le réviseur doit se concentrer sur l'attention de l'auteur du code dans le premier ou le deuxième commentaire, sans commenter chaque entrée, et l'auteur doit analyser le code pour le même type d'erreurs avant de publier la correction.
- Félicitez les décisions prises par l'auteur et les suggestions de l'examinateur.
- Laissez des commentaires sur les modifications apportées à la demande d'extraction elle-même, et non sur les validations individuelles. Ainsi, toute la correspondance sera au même endroit, y compris après le rebasage.
- Répondez aux commentaires dans les mêmes fils de discussion où ils ont commencé. Si le commentaire fait référence à la demande d'extraction entière ou à plusieurs commentaires dans le même fil, citez le commentaire commenté. Afin de passer d'un e-mail à un fil de discussion obsolète, au lieu du lien «l'afficher sur GitHub», utilisez le lien «dans chemin / vers / fichier».
- Si, au cours du processus de révision, des décisions cardinales sont prises du point de vue des normes de codage ou d'autres accords formalisés, l'auteur du code est tenu de noter ces décisions dans les documents pertinents.
- L'examen ne concerne pas seulement le code, mais l'ensemble de la tâche. Ne traitez pas les exigences de la tâche ou des mises en page comme la vérité ultime - tout le monde fait des erreurs et personne ne peut prendre en compte toutes les nuances. Un regard neuf et des suggestions judicieuses ne profiteront qu'au produit.
Processus d'examen par l'auteur du code
- La tâche n'a pas été achevée jusqu'à ce qu'elle ait passé l'examen, les tests et la sortie sur le "prod".
- Répondez à tous les commentaires du réviseur, ne laissez pas de commentaires sans réponse, que vous les acceptiez ou non.
- Réfléchissez aux questions qui se posent ou peuvent se poser pendant l'examen. Peut-être qu'il manque un commentaire à la section de code ou qu'il est préférable d'écrire le code plus explicitement.
- Ne corrigez pas les validations existantes avec un commit gend --amend; apportez plutôt des modifications en tant que validations distinctes, une pour chaque correctif. La correction des validations existantes complique considérablement le processus de révision, car vous devez revoir tout le code.
- Après avoir ajouté un nouveau commit à la demande d'extraction, écrivez dans un commentaire séparé un résumé des modifications afin que les parties intéressées soient au courant. Cela s'applique aux corrections lors de l'examen, ainsi qu'aux corrections lors des tests.
- Après la révision, avant d'envoyer la tâche pour test, réécrivez l'historique des validations (git rebase --interactive), apportez des corrections aux validations individuelles et supprimez les validations temporaires. Découvrez l'option --autosquash pour simplifier le processus.
Processus d'examen par l'examinateur
- Si vous êtes occupé au moment de la demande de révision, faites-moi savoir exactement quand vous commencerez la révision.
- N'exigez pas, mais suggérez d'apporter des changements.
- Commentez le code, pas la personne qui l'a Ă©crit.
- Vous assumez la responsabilité du code écrit, approchez l'examen en conséquence, mais cela ne signifie pas que l'auteur du code doit se conformer strictement à toutes vos exigences. N'oubliez pas que beaucoup de choses peuvent être faites de différentes manières.
- Suivez les objectifs de l'examen et proposez des modifications substantielles. N'insistez pas sur des changements non critiques. Indiquez l'importance des corrections dans un commentaire.
- Si vous rencontrez un problème grave, suspendez la révision et attendez que l'auteur le corrige. Très probablement, après la correction, il sera encore nécessaire de réviser à nouveau.
- Faites attention non seulement à ce qui a été changé, mais aussi à ce qui n'a pas été changé. Recherchez et montrez à l'auteur le code qui aurait dû être changé, mais qui ne l'a pas été (importations ou classes inutilisées oubliées, utilisation du code refactorisé ailleurs, etc.).
- Après avoir apporté des modifications par l'auteur du code, passez en revue les corrections et réexaminez la demande d'extraction dans son intégralité. Peut-être, sous réserve de corrections, vous aurez de nouveaux commentaires ou questions.
- Ne perdez pas de temps à revoir le code qui n'a pas changé dans le cadre de la tâche. L'allocation d'une partie du code dans une fonction est considérée comme un changement. Le transfert de code «tel quel» n'est pas considéré comme un changement. La réforme du code dans le fichier concerné est à la discrétion de l'auteur.
- Le réviseur n'édite pas indépendamment le code dans la branche, car il le veut tellement ou plus facilement. Les modifications sont apportées par l'auteur du code.
- Si vous avez entrepris une révision, essayez de ne pas retarder ou passer à d'autres tâches au détriment du courant
Matériaux utilisés et liens connexes
PS Partagez vos règles, recommandations et cas d'utilisateurs utiles sur le processus de révision dans les commentaires