Gerrit/Relecture de code/Ajouter des relecteurs
- Pour apprendre comment relire le code des autres contributeurs consultez le tutoriel et le Guide de la relecture de code.
Comment faire pour que vos modifications de code soient relues plus rapidement et aient plus de chance d'être acceptées ?
Prérequis
- Vous avez modifié du code pour corriger un bogue ou ajouter une nouvelle fonctionnalité.
- Vous avez suivi les Conventions de codage.
- Vous avez suivi la Liste des contrôles à faire avant la validation.
- Vous avez suivi les Points de sécurité vérifiés par les développeurs.
- Vous devez utiliser un Compte développeur avec Gerrit et configurer Git / Gerrit sur votre machine avec succès.
Maintenant vous envisagez d'écrire une correction à téléverser dans Gerrit pour que son code soit relu et fusionné (c'est à dire intégré à la base de code).
Votre correction
Vérifier d'abord que la correction répond au sujet
Si le code que vous proposez ne corrige pas un bogue mais qu'il introduit une nouvelle fonctionnalité, parlez-en d'abord avec le mainteneur pour être sûr que votre idée est conforme avec les objectifs du projet et que l'approche technique que vous proposez est la solution optimale.[1] Ceci économise votre temps et ...une déception potentielle.
Testez vos modifications
Testez vos modifications dans votre environnement de développement pour vérifier qu'il n'y a pas d'erreurs de compilation ni de tests en échec. Si vous n'avez pas testé votre patch pour une raison quelconque, dites le explicitement dans un commentaire de Gerrit. Suivez ensuite la Liste des contrôles à faire avant la validation.
Evitez de fournir des corrections incomplètes et d'introduire de nouveaux bogues.
Si vous savez déjà que votre correction a besoin de plus de travail, dites le explicitement dans Gerrit avec un -2 lors de la relecture ou en préfixant le message de validation (commit) avec [WIP] (work in progress).
Faites des validations courtes
Des corrections petites, indépendantes et complètes ont plus de chance d'être acceptées.[2][3] Plus il y a de fichiers à relire dans vos corrections et plus le relecteur devra y consacrer son temps et ses efforts[4] et il faudra reitérer les relectures.[5]
Si vos validations concernent plusieurs fichiers qui ne sont pas fortement dépendants, il est préférable de les séparer en les regroupant en plusieurs validations plus petites.
En plus, assurez-vous de ne pas inclure de modifications inutiles dans votre correction (par exemple en corrigeant le style du code).[1]
Néanmoins, si vos validations se répètent sur les mêmes fichiers, rassemblez-les en une grande validation (en utilisant soit --amend ou en les écrasant après cela).
Rédigez un message de validation significatif
Le message de validation doit expliquer quoi ? et pourquoi ?. Quel était le problème ? Comment la correction résout-elle le problème ? Comment tester qu'elle fonctionne effectivement ? Voir Gerrit/guide des messages Commit pour plus d'informations.
Egalement, assurez-vous de relire et d'utiliser dans votre message de validation, le vocabulaire adapté et la ponctuation.
Fournissez la documentation
Si une fonctionnalité dans votre correction doit être visible des utilisateurs finaux ou des administrateurs, vérifiez la mise à jour ou créez la documentation associée.[1] Voir la Politique de la documentation pour plus d'informations.
Ne mélangez pas les rebase avec les modifications
Si vous devez rebaser, ne faites que le rebase. Si des modifications qui ne concernent pas le rebase sont faites dans une action de rebase de corrections, vous devrez relire beaucoup plus de code pour les retrouver, et cela n'est pas du tout évident. Lorsque vous faites une modification importante, laissez un commentaire Gerrit pour expliquer son action et relisez le résumé de la validation pour vérifier qu'il est encore actuel.
Votre activité
Soumettre un ticket Phabricator
Create a task in Phabricator explaining the motivation (see Phabricator/Help). Patches without Phabricator tickets are harder to find reviewers for. To associate them, include a line in the commit message that says "Bug:" followed by the Phabricator ticket number.
Répondez aux tests en faute et aux commentaires
Vérifiez vos paramètres Gerrit et assurez-vous de recevoir les notifications par courriel. Si votre code échoue aux tests automatisés, ou que vous avez déja été relu, répondez dans un commentaire ou soumettez à nouveau.
(Pour connaître la cause de l'échec des tests automatiques, cliquez sur le lien du commentaire failed dans Gerrit, survolez avec la souris le point rouge du test en échec, attendez que la fenêtre de popup apparaisse, puis cliquez sur la sortie console (console output).)
Les commentaires viendront habituellement (mais pas toujours) de Gerrit avec une marque C-2, C-1, C+1, ou C+2. Vous trouverez les détails de ces explications (du point de vue du relecteur) sur Finaliser la relecture de code. Certains relecteurs peuvent proposer des suggestions d'amélioration sans utiliser explicitement un C-1 pour rassurer les nouveaux contributeurs. Vous devez encore prendre sérieusement en considérations leurs suggestions.
Quelques fois vous recevrez des appréciations que vous considérerez comme inadaptées, par exemple ayant trait à l'esthétique du code. Ne les ignorez pas mais reprenez votre correction pour répondre aux demandes triviales : vous pouvez être en désaccord mais en discutant vous perdrez plus de temps que si vous aviez accepté le point.
La relecture du code est une partie de la construction du consensus, et non pas une course aux élections. Vous êtes supposé répondre à un commentaire négatif, n'essayez pas de le contredire en apportant des commentaires positifs supplémentaires. Répondre à un commentaire négatif n'oblige pas toujours de modifier sa correction : quelques fois une meilleure explication est suffisante. Néanmoins même dans ces cas il peut être utile d'incorporer ces explications dans le message de validation ou dans les commentaires du code pour mieux informer les mainteneurs futurs à propos du même problème.
En général : soyez patient et grandissez un peu ! Les collaborateurs les plus expérimentés qui soumettent leurs corrections reçoivent plus rapidement des réponses positives.[5]
Ajoutez des relecteurs
Le choix des relecteurs joue un rôle important dans le temps de relecture. Les relecteurs les plus actifs fournissent les réponses les plus rapides.[5]
Juste après votre validation ajoutez un ou deux développeurs à la correction pour la relecture. (Ce sont des demandes – il n'y a pas de moyen pour assigner une relecture à une personne particulière dans Gerrit.) Les développeurs expérimentés devraient aider à ce propos : si vous observez qu'il reste des modifications non relues, ajouter leur des relecteurs. Pour trouver des relecteurs :
- Regardez la liste principale des mainteneurs, ou les mainteneurs listés sur la page de l'extension, afin de trouver qui s'occupe actuellement de la maintenance de cette partie de code, ou qui est en formations pour la maintenance.
- On your Gerrit patch's page, click on the project name, or "Repo" (e.g. mediawiki/core). Puis cherchez les autres corrections de ce dépôt : les personnes qui écrivent et qui relisent ces corrections sont de bons candidats à ajouter comme relecteurs.
- In large or very active projects, try filtering this list further. For example, if you're working on the MediaWiki database code, search for "message:rdbms" to find other changes in the rdbms component (this works as long as everyone follows the commit message guidelines and includes the component name in their commit messages).
- You can also filter changes by the files they modify; for example, search for file:mediawiki.page.gallery.styles to find all changes to styles of galleries, or path:resources/src/mediawiki.page.gallery.styles/print.less to narrow your search down to just their print mode styles (you can copy the file paths from your Gerrit patch's page, using the "Copy to clipboard" icon
on the file listing). - See Gerrit search documentation for more advanced options.
- On your Gerrit patch's page, if you view the changes in one of the files by clicking on it in the listing at the bottom, you can click "Show blame" (in top-right corner) to find the previous patches affecting each line of the file. Unlike the previous methods, this often works even if the file has been renamed or if the code has been moved from another file. The authors and reviewers of those patches may be willing to review your work too.
Relisez davantage de corrections
Plusieurs yeux rendent les bogues moins difficiles. Lisez le Guide de la revue de code et aidez les autres auteurs en approuvant ou en critiquant leur validations. Les commentaires restent indépendants, ils ne provoqueront pas les fusions ni les rejets et n'ont pas d'effet formel sur la relecture du code. Mais en relisant vous allez apprendre, gagner en réputation, de sorte que les relecteurs eux-mêmes viendront relire votre code à l'avenir, vous redonnant ainsi la pareille. "How to review code in Gerrit" has the step-by-step explanation.
Comment résoudre les obstacles éventuels
Pas de commentaire temporisé
Les ressources humaines dans les projets logiciels à source libre sont limitées et les intérêts des développeurs peuvent évoluer. Certains dépôts de code sont plus actifs et maintenus que d'autres et vous recevrez les relectures plus rapidement. D'autres domaines ont une politique de maintenance pas très bien définie, ou même ils ont été abandonnés et vous pourriez encore attendre longtemps.
Vous pouvez voir les dernières activités du dépôt de code via git log sur votre copie locale.
Pour reprendre en main un projet abandonné et devenir son mainteneur, suivez ces étapes.
Si vous pensez que votre patch est passé inaperçu depuis un long moment, n'hésitez pas à signaler le problème sur le canal IRC #wikimedia-tech connecter.
Autres raisons pour restructurer le code ou le rejeter
Même si vous avez suivi toutes les recommandations, il est possible que votre correction nécessite encore d'être restructurée (ou dans de rares cas elle peut même être rejetée).
Outre ce qui a déjà été mentionné, il existe d'autres raisons potentielles de rejet (elles ne sont pas toutes également décisives), telles qu'une solution sous-optimale alors qu'il existe un moyen plus simple ou plus efficace, les problèmes de performance, de sécurité, une dénomination améliorable (par exemple le nom des variables), les conflits d'intégration avec le code existant, la duplication du travail, une (mauvaise) utilisation involontaire de l'API ou des modifications proposées aux API internes considérées comme étant trop risquées.[1]
Ayez à l'esprit que les points de vue peuvent être différents : les relecteurs des corrections attribuent souvent plus d'importance que les auteurs aux tests en échec, aux corrections incomplètes, à l'introduction de nouveaux bogues, aux solutions sous-optimales ou à la documentation incohérente pour décider du rejet.[1]
Voir aussi
- 5 conseils pour que votre code soit relu plus rapidement
- Notes des réunions de relecture du code - depuis 2012; les parties concernées sont dorénavant incluses dans la présente documentation
Références
- ↑ 1.0 1.1 1.2 1.3 1.4 Yida Tao; Donggyun Han; Sunghun Kim, "Writing Acceptable Patches: An Empirical Study of Open Source Project Patches," in Software Maintenance and Evolution (ICSME), 2014 IEEE International Conference on , vol., no., pp.271-280, Sept. 29 2014-Oct. 3 2014
- ↑ Peter C. Rigby, Daniel M. German, and Margaret-Anne Storey. 2008. Open source software peer review practices: a case study of the apache server. In Proceedings of the 30th international conference on Software engineering (ICSE '08). ACM, New York, NY, USA, 541-550.
- ↑ Peter Weißgerber, Daniel Neu, and Stephan Diehl. 2008. Small patches get in!. In Proceedings of the 2008 international working conference on Mining software repositories (MSR '08). ACM, New York, NY, USA, 67-76.
- ↑ Amiangshu Bosu, Michaela Greiler, and Christian Bird. 2015. Characteristics of useful code reviews: an empirical study at Microsoft. In Proceedings of the 12th Working Conference on Mining Software Repositories (MSR '15). IEEE Press, Piscataway, NJ, USA, 146-156.
- ↑ 5.0 5.1 5.2 Baysal, O.; Kononenko, O.; Holmes, R.; Godfrey, M.W., "The influence of non-technical factors on code review," in Reverse Engineering (WCRE), 2013 20th Working Conference on , vol., no., pp.122-131, 14-17 Oct. 2013