Flux de travail de révision de code où l'auteur de la demande d'extraction doit fusionner

16

Plusieurs équipes de mon entreprise pratiquent un workflow de révision de code que je n'avais jamais vu auparavant. J'essaie de comprendre la pensée derrière cela, avec l'idée qu'il est utile de rendre l'ensemble de l'entreprise cohérent. (Je contribue à plusieurs bases de code et j'ai été trébuché par les différences dans le passé.)

  1. L'auteur du code soumet une demande d'extraction
  2. Le réviseur examine le code
    • Si le réviseur approuve, il laisse un commentaire du type "Ça a l'air bien, n'hésitez pas à fusionner"
    • Si le réviseur a des préoccupations, il laisse un commentaire du type "Veuillez corriger les problèmes mineurs X et Y, puis fusionner" (pour les modifications majeures, revenez à l'étape 2)
  3. L'auteur du code apporte des modifications si nécessaire, puis fusionne sa propre demande d'extraction

J'ai les préoccupations suivantes:

  • Dans le cas de l'approbation à l'étape 3, ce flux de travail crée un aller-retour apparemment inutile pour l'auteur de la demande d'extraction. Le réviseur, qui examine déjà le code, pourrait simplement le fusionner immédiatement.

  • Dans le cas où des modifications sont demandées à l'étape 3, l'agence de fusionner la demande d'extraction appartient désormais uniquement à l'auteur du PR. Personne d'autre que l'auteur ne se penchera sur les changements avant la fusion.

Quels sont les autres avantages ou inconvénients de ce flux de travail? Ce flux de travail est-il courant dans d'autres équipes d'ingénierie?

aednichols
la source
5
Avez-vous demandé aux membres de votre organisation pourquoi ils ont choisi ce flux de travail spécifique? Ils sont probablement mieux placés que nous pour éclairer les mérites pertinents. Nous ne ferions que spéculer.
Robert Harvey
1
Que se passe-t-il dans votre organisation lorsque le réviseur écrit "Veuillez corriger le problème majeur X"?
Doc Brown
8
D'après mon expérience, il est préférable que l'auteur d'origine soit celui qui effectue la fusion au cas où un conflit de fusion serait résolu. L'auteur d'origine est généralement celui qui est le mieux équipé pour comprendre comment résoudre un conflit de fusion.
17 du 26
Je serais curieux de savoir la logique ici. Vous devriez demander à vos collègues et l'écrire comme une réponse personnelle - il pourrait y avoir un très bon processus de réflexion ou une justification ici. Je ne suis tout simplement pas en mesure d'en trouver un rapidement.
Thomas Owens

Réponses:

21

Dans le premier cas, c'est généralement une courtoisie. Dans la plupart des organisations, les fusions lancent une série de tests automatisés qui doivent être traités rapidement en cas d'échec. Surtout s'il y a eu un délai important entre le moment où une demande de retrait a été soumise et le moment où elle a été examinée, il est poli de permettre qu'elle soit fusionnée selon le calendrier de l'auteur, afin qu'ils aient le temps de faire face aux retombées inattendues. La façon la plus simple de le faire est de les laisser fusionner eux-mêmes.

De plus, parfois, l'auteur prend conscience des raisons pour lesquelles une demande d'extraction ne doit pas encore être fusionnée. Peut-être que le RP d'un autre développeur est plus prioritaire et provoquerait des conflits. Peut-être qu'elle a pensé à un cas d'utilisation découvert. Peut-être qu'un commentaire de révision a déclenché un remue-méninges qui nécessite une enquête plus approfondie avant que le problème ne soit pleinement satisfait. L'auteur en sait le plus sur le code, et il est logique de lui donner le dernier mot sur sa fusion.

Sur le deuxième point, c'est juste une question de confiance. Si vous ne pouvez pas faire confiance aux gens pour résoudre des problèmes mineurs sans être revérifiés, ils ne devraient pas travailler pour vous. Si le problème est suffisamment important pour nécessiter un autre examen après la correction, faites confiance aux examinateurs pour en demander un.

Cela étant dit, je fusionne parfois les demandes de tirage d'autres auteurs, mais il s'agit généralement de modifications très simples ou de sources externes, où je prends personnellement la responsabilité de gérer les échecs d'automatisation des tests.

Karl Bielefeldt
la source
2
On dirait qu'il y a plus de variété que je ne l'avais imaginé. D'après mon expérience avec les tests automatisés, les tests s'exécutent sur la branche avant sa fusion, et non après, devenant ainsi une condition préalable à la fusion. J'ai également vu des correctifs "mineurs" non révisés, y compris le mien, qui provoquent des bogues.
aednichols
2
Les tests s'exécutent souvent à la fois comme une condition préalable et une condition préalable. Il est tout à fait possible que des modifications de plusieurs branches entrent en conflit de manière non évidente qui ne se présenterait pas comme un conflit de code et ferait échouer les tests. Sur mon lieu de travail, nous exigeons qu'une branche soit à jour avec la branche de base ainsi que tous les tests réussis avant qu'elle ne soit candidate à la fusion et que la fusion se fasse automatiquement si ces deux conditions sont remplies. Nous n'avions pas toujours cette première condition - avant cela, nous avions en fait rarement des problèmes à maîtriser malgré chaque branche passée individuellement
Matthew Scharley
3

La fusion de leur propre demande d'extraction par l'auteur initial est mon flux de travail préféré dans les petites équipes. En plus des avantages techniques déjà mentionnés (en termes de résolution des conflits de fusion, par exemple), je pense que cela ajoute de la valeur au niveau culturel: il crée un sentiment d'appartenance.

J'ai spécifié l' auteur initial pour le (rare) cas où un autre développeur ajouterait des validations à la demande d'extraction ouverte (en tirant la branche de fonctionnalité en cours et en la repoussant). Cela n'arrive pas souvent et résulterait d'une conversation en personne ou sur Slack: ces commits supplémentaires (par quelqu'un d'autre) ne devraient pas y atterrir par surprise! Dans ce contexte, par auteur initial , je veux dire celui qui a soumis la demande de retrait.

mkcor
la source
2

Dans mon organisation, nous sommes tout à fait nouveaux dans les demandes de tirage et votre question est celle que j'ai moi-même réfléchie.

Je voudrais ajouter une observation: dans certains outils (nous utilisons TFS), il peut y avoir un élément de travail associé à la demande d'extraction.

Si tel est le cas, cela devient un peu compliqué de savoir quand le réviseur a effectué la fusion. Dans ce scénario, le développeur doit alors revoir le PR, ouvrir le bogue ou la demande de changement et le marquer comme «résolu». Si nous le marquons «résolu» trop tôt, les testeurs pensent que le correctif fait déjà partie de la version actuelle.

TFS 2017 a amélioré sa mise en œuvre de la demande de pull. Désormais, le développeur peut demander que la demande Pull soit fusionnée automatiquement si toutes les conditions sont remplies (pas de conflit de fusion, approbation des réviseurs et pas de build cassé). YMMV.

9Rune5
la source
1

De cette façon, l'auteur a la possibilité de changer d'avis sur sa branche, peut-être qu'il a compris quelque chose qui devrait être fait d'une manière différente, et a remis les frais supplémentaires à l'examen.

gnasher729
la source