Quand vous proposez un patch, il ne sera intégré que lorsqu'il y aura eu l'approbation d'au moins deux développeurs : un reviewer et un super-reviewer. Et cela concerne les patchs de n'importe quel contributeur, que ce soit un contributeur lambda occasionnel, ou que ce soit l'un des "core-developer" qui connaît le coeur de Gecko sur le bout des doigts. Personne n'y échappe. Toutefois, pour certain patch de core-developer, il arrive qu'il n'y ait qu'une seule review d'un autre core-developer, pour la simple et bonne raison que ce qu'ils font parfois n'est maîtrisé/compris que par peu de personne, tellement le code est complexe :-). Mais ce sont des exceptions qui confirment la règle.

Un "reviewer" va examiner votre patch et cherchera la petite bête :

  • Trouver éventuellement les portions de code mal faites : peu optimisée, mauvais algorithmes, bugs éventuels, trous de sécurité éventuels etc..
  • Respect de la norme d'écriture du code de Mozilla (coding style) : ils sont très à cheval là dessus.

Je vois au moins trois avantages à ce système :

  1. Le premier point que l'on peut noter, c'est que ça oblige un contributeur à faire preuve d'une certaine humilité. On peut penser que notre patch est bien fait, mais il arrive que des reviewers nous ramène à la réalité, pointant du doigt les problèmes du patch. Et quand un reviewer refuse le patch (toujours en donnant les raisons et en indiquant ce qui ne va pas), il faut mettre de coté son orgueil et se remettre au travail. Se remettre ainsi en cause permet de progresser.
  2. Le deuxième point, c'est que cela garantie une certaine robustesse : avoir plusieurs paires d'yeux qui analyse le même bout de code permet de déceler plus facilement les bugs, et ceci avant même que ces bugs ne soient découverts une fois que le patch est intégré officiellement.
  3. Enfin le code reste toujours lisible, compréhensible grâce au respect du coding style.

Bien sûr tout ceci n'est pas parfait, l'être humain n'étant pas parfait, des bugs passent au travers de ces "filtres". Mais c'est un processus que je trouve bien pensé.

À noter aussi certaines choses pratiques :

  1. tout patch (et toutes ses versions) est attaché dans un bug dans bugzilla
  2. tout commentaire (review ou pas) à propos de ce patch est enregistré aussi au niveau du bug dans le bugzilla
  3. les dépendances entre bugs sont indiquées

Cela peut paraître insignifiant, mais j'y ai vu de nombreux avantages : tout est centralisé, on n'a pas à chercher à plusieurs endroits en même temps pour retrouver l'historique d'un bug : qui a fait le patch, quel est ce patch, qui a fait la review, qui a commité, quand, pourquoi le bug a été ou non résolu, si le bug a été résolu grâce au patch d'un autre bug etc.

À l'opposé par exemple du modèle de développement de PHP (un autre que je pense assez connaître), où les informations au niveau d'un bug sont succinctes, où il faut aller rechercher dans une mailing list le patch qui correspond à un bug, et dans une autre mailling list les discussions à propos de la résolution de ce bug. Bref, un véritable parcours du combattant pour avoir une information aussi triviale que de savoir sur quel branche par exemple le bug a été résolu. J'ai vu aussi souvent des bugs fermés sans aucune explication, ni justification alors que le bug en question énonçait un réel problème. J'ai d'ailleurs trouvé que certains de ces développeurs qui ferment les bugs, avaient une attitude particulièrement hautaine envers les contributeurs (attitude que je n'ai pas trouvé chez les dev de Mozilla), accentué par le fait qu'ils utilisent souvent des réponses pre-écrites. Le summum du mépris envers le contributeur. Bref, ça me tente vraiment pas de contribuer à PHP (malgré que j'utilise ce langage assez intensivement et qu'il y ait des choses que j'aimerais bien voir s'améliorer).