Quelle est la meilleure façon de revoir un code avant qu'il ne soit validé dans le tronc? (SVN)

Quelle est la meilleure façon d'examiner un code avant qu'il ne soit validé dans le tronc SVN? Une idée à laquelle je pense est de demander au développeur de valider son code dans une branche, puis de revoir son code tout en fusionnant les révisions de la branche dans le tronc. Est-ce une bonne pratique? Sinon, que pourrait-on faire d'autre pour revoir un code avant qu'il ne soit validé dans le tronc?

Il y a cependant deux écoles - ce que vous proposez ou "révisez avant de vous engager". La plupart des différences peuvent être considérées comme négatives et / ou positives. - par exemple, aucun suivi des changements causés par un examen - voulez-vous les voir comme des engagements discrets, ou êtes-vous uniquement intéressé par le travail final?

Révision avant validation - aucune branche requise (bien que cela puisse être fait si vous le souhaitez), doit donner aux réviseurs l'accès aux dossiers de travail. Le code peut être modifié pendant et après la révision sans suivi. Les corrections causées par la révision n'apparaissent pas dans le référentiel.

Révision après validation (sur une branche) - Besoin de faire tourner une branche pour chaque révision (bien que cela puisse déjà être dans le flux de travail). Le code soumis pour examen ne peut pas être modifié sans suivre les modifications. Quelqu'un doit fusionner les branches examinées et garder une trace de ce qui a été examiné et de ce qui ne l'a pas été.

Cela dépend en grande partie de la culture et de l'expérience de l'équipe. Quel est votre modèle de confiance, quel est le principal objectif des avis? Personnellement, je préfère la révision après validation, car elle permet de suivre les modifications résultant de la révision. Nous utilisons maintenant Git et Gerrit car ils fournissent un bel équilibre entre les deux options.

1. Avant de valider, exécutez «svn diff» pour générer un fichier correctif.
2. Envoyez le fichier de correctif au réviseur qui l'applique à une copie propre du tronc.
3. Le réviseur passe en revue les modifications en utilisant le visualiseur de différences de son choix.
4. Le réviseur effectue la construction et exécute des tests.
5. Si tout se passe bien, dites au développeur qu'il peut vérifier ses modifications. S'il
   y a des problèmes, le développeur revient à l'étape 1.

Il y a le monde idéal, puis le monde réel.

Dans le monde idéal , tout votre code est testé, vous pouvez donc être sûr que tout ce qui est archivé fonctionnera ou vous saurez qu'il est cassé car il échoue à un ou plusieurs tests. De plus, toute personne moins expérimentée sera jumelée avec quelqu'un qui est expérimenté, donc la révision du code se fait à la volée (vous vous engagez au fur et à mesure).

Dans le monde réel , les choses sont différentes. Les entreprises veulent que ce changement vive maintenantet vous dira, avec un visage parfaitement droit, que oui, vous aurez le temps de nettoyer le code et d'ajouter des cas de test plus tard. Vous n'aurez probablement pas le temps de tout réviser et le pourcentage de code couvert par les tests diminue continuellement. La principale raison de la révision du code est que les développeurs juniors apprennent des développeurs seniors (quand il y a du temps pour cela) en demandant à une personne plus expérimentée d'examiner les changements et de suggérer "de meilleures façons de faire les choses". Vous aurez des développeurs seniors qui ne font que valider du code non révisé. Branchement juste pour un examen du code, puis la fusion est une énorme perte de temps. Une façon de surmonter ce problème consiste à déclarer une réunion d'équipe hebdomadaire régulière de 2 heures (ou plus) au cours de laquelle vous choisissez un ou deux changements récents sur lesquels les personnes ont travaillé à brève échéance et demandez à leurs auteurs de "présenter". leur approche en regardant le code ensemble sur un projecteur ou quelque chose. Cela peut conduire à des discussions intéressantes (généralement hors sujet un peu) mais améliore généralement la compréhension de chacun sur la façon de le faire correctement. De plus, la pression de devoir éventuellement présenter votre code fait que certaines personnes le font mieux ;-)

Ou vous pourriez avoir de la chance et vous mettre au travail dans un environnement réel où ce n'est pas si agité, les programmeurs sont réellement appréciés pour ce qu'ils font au lieu d'être maltraités, et il est temps de tout faire correctement. Dans ce cas, ma réponse serait: essayez quelques-unes des différentes méthodes suggérées dans les réponses ici et voyez celle qui convient à votre équipe et la façon dont vous travaillez le mieux.

Les succursales devraient fonctionner correctement, d'après mon expérience de leur utilisation dans les évaluations pré-validées lors d'un travail antérieur.

_{Notez qu'à l'époque, nous n'utilisions les révisions avant validation que pour les correctifs critiques du code candidat de la version de production, donc il n'y avait pas beaucoup de branches (les modifications de routine étaient passées par les révisions post-validation).}

Comme vous semblez utiliser des révisions avant validation pour toutes les modifications, vous devrez peut-être gérer un grand nombre de branches. Si vous vous attendez à ce que le développeur effectue un changement "révisable" par semaine en moyenne, vous finirez par avoir environ 50 succursales chaque année pour chaque développeur de l'équipe. Si vous utilisez de plus petits morceaux de travail - comme ceux qui prennent 1, 2, 3 ... jours - multipliez 50 par 2, 3, 5 ... en conséquence.

Voici quelques autres considérations à prendre en compte si vous le souhaitez de la meilleure façon .

1. gestion des cas lorsqu'un examen retardé bloque le code nécessaire pour les autres membres de l'équipe

Établissez, surveillez et résolvez les conflits liés aux délais de révision du code. Si je me souviens bien des examens préalables à la validation des changements de routine que j'ai traités dans l'un des projets précédents, le délai raisonnable est d'environ 3 jours et le temps de commencer à s'inquiéter est lorsque l'examen n'est pas terminé plus d'un jour après la soumission.

_{À titre de comparaison, lors des revues post-commit, ces exigences sont beaucoup plus détendues (j'utilise un délai de 2 semaines et je commence à m'inquiéter après 1 semaine) - mais puisque vous ciblez les revues pré-commit, ce n'est probablement pas intéressant.}

2. fusionner les conflits lors de la soumission du code révisé

Que faire si la validation du code révisé est bloquée par des modifications conflictuelles validées par quelqu'un d'autre pendant que le code attendait la révision?

Quelques options à considérer sont

• revenir au début et demander aux développeurs de réimplémenter et de revoir le changement
  Dans ce cas, vous devrez peut-être remédier à un impact négatif sur le moral de l'équipe que cela pourrait (aura!) avoir.
• céder la responsabilité de la fusion à un autre membre de l'équipe («maître de fusion»)
  Dans ce cas, vous devrez également décider si les fusions en elles-mêmes doivent passer par un examen préalable à la validation ou non - et si oui, que faire dans le cas où cette fusion rencontre à son tour un autre conflit.
• ignorer les modifications apportées au code révisé au stade de la fusion
  Dans ce cas, vous devrez peut-être remédier à un impact négatif sur le moral de l'équipe lié au fait que le code engagé diffère de celui qui a été examiné.
• inventer un moyen d'éviter les conflits L'
  approche directe consiste à n'autoriser qu'un seul développeur à la fois à modifier un ensemble particulier de fichiers - bien que cela ne vous protège pas du type de changements qui ne modifient pas directement les fichiers, mais les impactent par des changements d'API internes . Vous pourriez également découvrir que ce type de «verrouillage pessimiste» rend les changements à l'échelle du système et le refactoring en profondeur assez gênants.

_{À titre de comparaison, il n'y aurait pas de problèmes de ce type dans les révisions post-validation (car elles traitent de code déjà fusionné par définition) - mais puisque vous ciblez les révisions pré-validation, ce n'est probablement pas intéressant.}

3. charger le développeur en attente d'examen

Établissez une politique explicite pour savoir si le développeur qui a soumis la révision doit passer à une nouvelle tâche ou faire autre chose (comme par exemple poursuivre le réviseur).

_{À titre de comparaison, les révisions post-validation n'ont guère besoin d'une politique explicite (car il est naturel de passer à la tâche suivante après avoir validé le code et en tenant compte du fait que la date limite de révision est d'une semaine ou deux) - mais puisque vous ciblez les révisions pré-validation, c'est probablement pas intéressant.}

Tout élément de développement nécessitant un examen devrait être dans une branche distincte. Ainsi, la succursale devrait déjà exister avant le moment de la révision. Ensuite, l'étape devrait être:

1. La revue
2. Réviser (éventuellement)
3. boucle de retour à l'examen (éventuellement)
4. Fusionner dans le coffre

La fusion est la partie difficile. Plus la branche restera indépendante longtemps, plus il sera difficile de la refondre dans le tronc. (Il pourrait également être plus difficile à tester.)

La fusion croisée est une solution possible. Avant de fusionner dans le tronc (étape 4, ou même plus tôt, disons avant l'étape 3 ou l'étape 1), fusionnez le tronc dans la branche. Le développeur peut le faire et le tester. Ensuite, la branche rattrape le tronc et il devient plus facile de la fusionner dans le tronc. Il est préférable que vous fusionniez dans le coffre ou par la personne responsable du coffre.

Certaines personnes essaient de rebaser au lieu de fusionner. Certaines personnes soutiennent que le rebase est mauvais. C'est un autre débat.