Vos meilleurs programmeurs devraient-ils vérifier le code de tout le monde dans le contrôle de code source?

29

L'une des différences entre svn et git est la possibilité de contrôler l'accès au référentiel. Il est difficile de comparer les deux car il y a une différence de point de vue sur qui devrait être autorisé à commettre des changements!

Cette question concerne l'utilisation de git comme référentiel centralisé pour une équipe d'une entreprise quelque part. Supposons que les membres de l'équipe ont des niveaux de compétences différents, comme ils le sont dans la plupart des entreprises.

Git semble supposer que seuls vos meilleurs programmeurs (les plus productifs et les plus expérimentés) sont autorisés à archiver le code. Si tel est le cas, vous prenez le temps d'écrire du code pour réviser le code des autres afin de l'archiver. Cela est-il payant? Je veux vraiment concentrer cette question sur la meilleure utilisation du temps de votre meilleur programmeur, et non sur les meilleures pratiques de contrôle de version en général . Un corollaire pourrait être: les bons programmeurs quittent-ils si une partie importante de leur travail consiste à revoir le code des autres? Je pense que les deux questions se résument à: l'examen vaut-il le coup sur la productivité?

GlenPeterson
la source
5
Définir "meilleur programmeur"? Le mieux dans quoi? Vous suivez des règles arbitraires? Lancer le code? Vous écrivez du code zéro défaut?
Timo Geusch
3
Désolé, j'essaie toujours de comprendre le concept de la révision du code non contrôlé (c'est-à-dire non enregistré) ... l'un des principaux avantages de l'utilisation d'un SCS est certainement que la révision peut être effectuée par rapport à un code connu / contrôlé. itération du code?
Andrew
2
@Andrew avec gitn'importe quel développeur peut avoir son propre référentiel (sur son ordinateur personnel) et un référentiel personnel public (celui sur un serveur, derrière apache) auquel il ne peut ajouter que des modifications. La différence est que seul le référentiel des développeurs principaux est le "béni", celui à partir duquel tout le monde doit passer à la caisse. Le code d'extraction de plomb provenant des référentiels publics du développeur et les fusionne avec son référentiel public. Vous avez tous les deux une itération connue / contrôlée ainsi qu'un contrôle de source à tout moment.
Hubert Kario
32
"Git semble supposer que seuls vos meilleurs programmeurs (les plus productifs et les plus expérimentés) sont autorisés à vérifier le code" est une présomption incorrecte. Git peut être configuré comme vous le souhaitez. Le modèle "Pull request" n'est qu'un moyen - idéalement adapté aux projets open source avec un grand nombre potentiel de contributeurs inconnus. Dans la plupart des environnements commerciaux, le modèle "pull request" serait un drapeau rouge, indiquant des processus et procédures SDLC et QC médiocres.
mattnz
4
Je pense que @mattnz est correct ici. Ceci est uniquement le résultat d'une forte influence open source sur git où il y a une équipe de développement principale qui contrôle l'état du dépôt, mais d'autres sont également invités à contribuer.
Steven Evers

Réponses:

53

Comme cela ne ressort pas clairement de votre question, je tiens simplement à souligner qu'un workflow de contrôleur d'accès n'est en aucun cas requis avec git. Il est populaire auprès des projets open source en raison du grand nombre de contributeurs non fiables, mais n'a pas autant de sens au sein d'une organisation. Vous avez la possibilité de donner à tout le monde un accès push si vous le souhaitez.

Ce que les gens négligent dans cette analyse, c'est que les bons programmeurs passent beaucoup de temps à traiter le code cassé des autres programmeurs de toute façon. Si tout le monde a un accès push, alors la construction sera cassée, et les meilleurs programmeurs ont tendance à être ceux qui intègrent et traquent fréquemment les coupables lorsque les choses se cassent.

La chose à propos de tout le monde ayant un accès push est que lorsque quelque chose se casse, tout le monde qui tire obtient une construction cassée jusqu'à ce que le commit incriminé soit annulé ou corrigé. Avec un workflow de contrôleur d'accès, seul le contrôleur d'accès est affecté. En d'autres termes, vous n'affectez qu'un seul de vos meilleurs programmeurs au lieu de tous.

Il peut s'avérer que la qualité de votre code est assez élevée et que le rapport coût-bénéfice d'un contrôleur d'accès n'en vaut toujours pas la peine, mais ne négligez pas les coûts habituels. Ce n'est pas parce que vous êtes habitué à cette perte de productivité qu'elle ne se produit pas.

N'oubliez pas non plus d'explorer les options hybrides. Il est très facile avec git de mettre en place un référentiel vers lequel n'importe qui peut pousser, puis de laisser un contrôleur d'accès comme un développeur senior, un testeur ou même un serveur d'intégration continue automatisé décider si et quand un changement en fait un deuxième référentiel plus stable. De cette façon, vous pouvez tirer le meilleur parti des deux mondes.

Karl Bielefeldt
la source
10
+1: Pour ... Les bons programmeurs passent de toute façon beaucoup de temps à gérer le code cassé des autres programmeurs.
Jim G.
3
+1 Meilleure réponse. Surtout en soulignant qu'un développeur commettant un bug de rupture de build a un impact négatif sur tout le monde.
Evan Plaice
Dans cette situation, il s'est avéré que les deux meilleurs programmeurs étaient utilisés à plein temps pour examiner et corriger le code des autres. Bien sûr, la qualité du code sur le VCS était bonne, mais le moral de ces deux personnes a diminué plus rapidement qu'une chasse d'eau. Ce qui a commencé comme une bonne idée en apparence s'est transformé en cauchemar lorsque ces deux-là ont couru vers des endroits où ils pourraient, disons, accomplir des tâches plus créatives.
Newtopian
1
C'est un bon point, @Newtopian. Les endroits où j'ai vu cela réussir ont plus un modèle de microservice, et une seule équipe Scrum a validé l'accès à un microservice donné, mais la responsabilité est répartie autour du système dans son ensemble. Si vous n'avez pas au moins deux programmeurs expérimentés par équipe Scrum, vos pratiques d'embauche doivent être améliorées.
Karl Bielefeldt
40

J'ai travaillé dans un emploi où les enregistrements étaient limités aux chefs d'équipe uniquement (et les chefs d'équipe ne pouvaient pas vérifier leur propre code). Cela nous a servi de mécanisme pour appliquer les révisions de code, en grande partie à cause d'un certain nombre d'incidents où de mauvais commits sont entrés dans la base de code, même autour des enregistrements bloqués et de l'analyse statique.

D'une part, il a fait son travail. Un certain nombre de mauvais commits ont été trouvés avant qu'ils n'entrent dans la base de code (et rapidement oubliés pendant une semaine environ jusqu'à ce que quelqu'un tombe sur eux). Cela a provoqué moins de perturbations dans la base de code. De plus, je pouvais repousser certaines choses de formatage / structure avant qu'elles ne deviennent une dette technologique; attraper quelques bugs avant qu'ils ne deviennent des bugs. Et cela m'a donné une excellente idée de ce que mon équipe faisait.

D'un autre côté, cela m'a fait entrer spontanément dans des rages meurtriers lorsque mon changement de 3 lignes a pris 4 heures à s'engager parce que je devais retrouver une autre piste et les amener à faire le commit. Cela m'a poussé à faire des commits beaucoup moins fréquents que les meilleures pratiques et a parfois conduit à des problèmes pour essayer de suivre les modifications apportées au développeur qui les avait effectuées.

Je ne le recommanderais généralement pas, sauf dans les environnements les plus nécessiteux. Faire les critiques et les commits n'était pas trop mal en fait. Cependant, avoir mon propre processus dépendant des caprices des autres était exaspérant. Si vous ne pouvez pas faire confiance à vos développeurs pour archiver le code, obtenez de meilleurs développeurs.

Telastyn
la source
8
@HubertKario - Si vos meilleurs développeurs passent du temps à réviser le code et que le reste est effectivement bloqué jusqu'à ce que ce soit terminé, je ne vois pas trop de différence pratique.
Telastyn
6
Comment sont-ils bloqués? Vous créez un correctif (validez localement), le soumettez en amont et continuez à travailler sur un nouveau widget (créez de nouvelles validations locales). Si votre modification est appliquée mot pour mot, il vous suffit de passer à la caisse et de fusionner le référentiel du prospect. S'il n'a pas été appliqué mot pour mot, vous pouvez toujours rebaser votre travail ultérieur. Si le changement est vraiment critique, vous pouvez le publier dans votre propre référentiel public et dire aux gens de le retirer à partir de là ou simplement de leur envoyer des correctifs. Dans ce cas git, détectera qu'une modification a déjà été effectuée et ignorera simplement l'application du correctif en amont spécifique.
Hubert Kario
9
La dernière ligne de cette question est vraiment le point à mes yeux. Un développeur auquel on ne fait pas confiance sera au mieux inefficace et au pire détestera son travail. N'embauchez pas de personnes en qui vous ne aurez pas confiance. Cela gaspille inutilement de l'argent sur des personnes pour lesquelles vous ne permettrez pas de faire le travail pour lequel vous les payez de toute façon.
Jimmy Hoffa
1
@HubertKario - vous savez mieux que moi. L'environnement dans lequel je me trouvais rendait ennuyeux de jongler avec les différentes branches / changesets.
Telastyn
5
@Telastyn Je ne sais pas si je suis censé interpréter votre réponse aussi littéralement que moi, mais un autre inconvénient serait l'historique des annotations / blâmes. Si vous trouviez du code que vous ne compreniez pas, vous finiriez par demander au réviseur qui l'a commis, pas au programmeur qui l'a écrit.
Daniel Kaplan du
28

Non. N'importe qui devrait pouvoir s'engager.

Si vous rencontrez des problèmes lors de la validation de bogues, ce n'est pas la politique de contrôle des sources qui est incorrecte. Ce sont les développeurs qui ne s'assurent pas que ce qu'ils commettent fonctionne. Donc, ce que vous avez à faire est de définir des directives claires sur ce qu'il faut engager et quand.

Une autre grande chose s'appelle les tests unitaires;)

Il existe cependant une alternative.

a) Si vous utilisez le contrôle de version distribué, vous pouvez créer un référentiel principal vers lequel seules les demandes d'extraction peuvent être effectuées. De cette façon, tous les développeurs peuvent obtenir le versioning de leur propre code pendant que vous contrôlez la branche principale.

b) En subversion et similaire, vous pouvez utiliser des branches où chaque développeur doit créer des correctifs pour le placer dans la branche principale.

jgauffin
la source
1
Cette. Si vous vous engagez sans tests unitaires et de build, avoir une exigence de révision de code est un bandage imparfait.
Brian Knoblauch
Ouais. C'est pourquoi j'ai mentionné les alternatives. Les révisions de code valent mieux que rien. Ne pas pouvoir versionner le code est une douleur à laquelle aucun développeur ne devrait être exposé.
jgauffin
2
Les tests unitaires ne sont pas utiles s'ils sont écrits par la même <insérez vos 4 lettres préférées ici> que le code unitaire.
ott--
@BrianKnoblauch: on pourrait dire que l'inverse est vrai aussi. Idéalement, vous devriez avoir les deux.
Doc Brown
@ ott-- Je viens d'entendre l'histoire d'un développeur qui est parti après avoir commis un horrible gâchis de correctif et commenté tous les assert dans ses tests unitaires. Les tests réussissent par défaut, il a donc fallu un certain temps pour remarquer le problème!
Alex
8

Vous devriez jeter un coup d'œil à des projets tels que Gerrit qui permet à tous les développeurs de pousser leur code dans la branche `` revue '' et une fois que vos développeurs seniors / principaux sont satisfaits de ces changements, ils peuvent les pousser en master / release.

S'ils ne sont pas satisfaits, ils peuvent laisser des commentaires à côté d'une ligne de code, demander un correctif mis à jour, etc.

De cette façon, toute personne avec un changement en attente peut le sortir dès qu'il est prêt et seules les personnes qualifiées (avec les bons privilèges +2 dans Gerrit) pourront pousser ce code à tester et plus tard à la production.

rochal
la source
2
Nous utilisons gerrit avec beaucoup de succès. Résout chaque problème avec lequel le PO a un problème et même certains qu'il ne sait pas avoir.
mattnz
8

Non, c'est une mauvaise utilisation de vos meilleurs talents. Imaginez une maison d'édition prenant ses auteurs les plus connus et les faisant éditer; mauvaise idée.

Il devrait y avoir des révisions de code, mais cela ne signifie pas que c'est toujours un Sr. vérifiant le code d'un jr. À terme, tous les membres de l'équipe devraient atteindre le niveau où ils peuvent contribuer au code avec un minimum de conseils. Ils passent par les trois niveaux de confiance:

  1. Aucun - je veux voir chaque ligne de code avant de le vérifier.
  2. Certains - Faites-moi savoir ce que vous faites et je vous ferai part de vos commentaires
  3. La plupart - Allez faire votre travail et ne demandez de l'aide qu'en cas de besoin.

Avantages de libérer votre talent:

  • se concentrer sur le design
  • participation à la mise en place de normes de codage et de stratégies d'application (sans tout faire manuellement)
  • s'attaquer aux problèmes de codage difficiles
  • offrir du mentorat (sans avoir approuvé chaque ligne de code)

Il y a des développeurs intéressés par un chemin de gestion qui peuvent préférer ne pas coder toute la journée; laissez les autres tranquilles.

JeffO
la source
1
+1. Laissez l'équipe évaluer l'équipe - le réviseur et le révisé peuvent tous les deux bénéficier, même si le réviseur est moins expérimenté que le révisé. Et vous pouvez faire tout l'examen APRÈS l'enregistrement. OMI, si vous empêchez les gens de s'enregistrer, leur productivité diminuera (malgré leur motivation).
Andy
5

l'examen vaut-il le coup sur la productivité?

Cela dépend de l '«équilibre» de l'équipe et de la façon dont les évaluations sont configurées. Les deux sont des questions de gestion et de travail d'équipe, aucune quantité de magie technologique de contrôle de version (centralisée ou distribuée) ne peut avoir une influence substantielle sur cela.

S'il est mal fait, la perte de productivité tuera bien sûr tous les avantages de l'examen; la réponse est cependant de ne pas laisser tomber l'idée des avis mais de savoir comment le faire correctement .

Une approche pour savoir si vos avis sont corrects consiste à utiliser l' outil de suivi des problèmes pour suivre le temps passé sur les avis (certains outils d'examen de code le permettent également). Si vous découvrez que les avis prennent beaucoup de temps, investissez des efforts pour trouver les raisons et les moyens d'améliorer les choses. De plus, cela ne ferait pas de mal d'avoir des entretiens 1: 1 réguliers avec les membres de l'équipe pour découvrir les problèmes potentiels avec les révisions de code.


Si les "meilleurs" programmeurs de l'équipe sont obligés de passer des heures à fouiller dans les ordures incompréhensibles produites par des codeurs merdiques, la solution consiste à licencier les fabricants de conneries, sans faire appel à la technologie VCS.

  • Dans l'un des projets précédents, j'ai été chargé d'examiner les modifications de code effectuées par un membre de l'équipe sous-performant en permanence, dans un composant qui a pris près d'une heure pour simplement générer et exécuter des tests. J'ai commencé à lire les différences et quand j'ai remarqué un changement incompilable, j'ai simplement terminé la révision, posté les commentaires nécessaires et demandé à la direction de s'assurer que les autres demandes de révision soient accompagnées d'une confirmation écrite de la compilation de leur code. Il n'y a eu aucune "demande de révision" depuis et bientôt le gars est parti.

D'un autre côté, lorsque l'équipe est raisonnablement équilibrée, les révisions de code sont amusantes et éducatives. Dans mon projet précédent, nous avions une exigence de révision à 100% du code et cela n'a pas pris beaucoup de temps ni distrait. Il y a eu des bogues découverts lors de l'examen, et il y a eu des débats sur le style de codage et les choix de conception, mais cela semblait juste ... normal .


Si les modifications de code sont bloquées pendant des jours ... des semaines avant d'arriver au contrôle qualité pour des tests "en raison de critiques", étudier les astuces VCS serait le moyen le moins probable de résoudre ce problème. Au lieu de cela, il serait préférable de concentrer leurs efforts sur la recherche de problèmes dans la façon dont le processus d'examen est organisé.

  • - Oh l'intégration de ce changement a été beaucoup retardée car le critique est tombé soudainement malade, quel malheur.
    - Salut! Hell-lo-oo, avez-vous déjà pensé à avoir des réviseurs de sauvegarde pour traiter des cas comme ça?
moucheron
la source
4

Oui. Mais seulement si vous parlez de contrôle de source distribué. Avec centralisé - cela dépend.

S'il n'y a que peu de programmeurs, cela prend peu de temps. Certainement moins que les correctifs qui seront nécessaires pour supprimer les bugs et les dettes techniques ultérieurement.

S'il y a de nombreux programmeurs, vous pouvez déléguer la tâche de révision du code aux lieutenants et demander au développeur principal de retirer (presque) sans aucun doute leurs modifications. Cela fonctionne pour le noyau Linux, je ne pense pas qu'il existe de plus gros projets logiciels ...

Encore une fois, si le projet est petit, le responsable verra rapidement qui donne du bon code et qui produit du mauvais code. Il verra assez rapidement que J.Random écrit un bon code qui n'a besoin que de vérifier les décisions architecturales tandis que le stagiaire écrit un mauvais code qui doit être revu ligne par ligne avant de fusionner. Les retours générés de cette façon réduiront le fardeau de la maintenance et donneront une expérience de première main sur tout ce que le stagiaire apprend réellement et doit être conservé en entreprise. Tirer et fusionner une branche d'un autre gitréférentiel prend littéralement une ou deux secondes, la lecture des titres des messages de validation prendra généralement plus de temps, donc après que je sache à qui faire confiance pour écrire du bon code, la fusion du code d'autres personnes n'est plus un problème.

Hubert Kario
la source
2

La révision du code ne nécessite pas nécessairement l'attention de vos meilleurs programmeurs. OMI, ce devrait être une chose informelle. Juste une deuxième opinion ou une deuxième paire d'yeux sur un morceau de code d'un non-débutant avant qu'il ne soit enregistré en production. Il aide à atténuer les oublis majeurs tout en aidant les gens à améliorer le codage en tant qu'artisanat en étant exposés à d'autres perspectives de développement.

Une sorte de lite de programmation de paires moins désagréable. En d'autres termes, cela ne devrait pas prendre longtemps et vous ne devriez pas avoir à attendre quelqu'un pendant des heures. Tout ce qui, dans votre processus de développement, implique que des personnes attendent des choses est un gaspillage d'argent et paralyse l'élan / le moral, OMI.

Si la révision du code était censée arrêter 99,5% des bogues avant qu'ils n'entrent dans votre base de code, il n'y aurait aucun intérêt réel à un contrôle de version sophistiqué. Cela dit, git est intimidant au début, mais l'utilisation générale de base n'est pas si compliquée et elle est hautement configurable. Vous devriez pouvoir vous arrêter pendant quelques heures pour apprendre à tout le monde à l'utiliser. Tout le monde s'engage. Toutes les recrues sauf les noobiest examinent jusqu'à ce qu'elles démontrent leur expertise dans quelque chose.

Erik Reppen
la source
0

Tant que les modifications soumises ont été examinées par les «meilleurs programmeurs», n'importe qui devrait être autorisé à soumettre du code. La seule personne qui devrait avoir la possibilité d'appliquer le contrôle sur un référentiel est l'ingénieur de publication, si cette personne existe.

Personnellement, je serais bien énervé si je devais vérifier le code des autres.

Quelques commentaires sur votre montage: non, ils ne devraient pas. Les critiques sont un mal nécessaire, elles font plus de bien que de mal et les bons programmeurs l'apprécieront. Peut-être y a-t-il une réticence à participer aux revues parce qu'ils n'aiment pas l'idée que des `` programmeurs mineurs '' critiquent leur code. C'est vraiment dommage. Ils seraient beaucoup plus susceptibles d'arrêter si la ligne de code est constamment boguée et ils passent plutôt leur temps à nettoyer après les soumissions à moitié cuites d'autres personnes.

James
la source
0

Oui, l'examen en vaut la peine. Je ne suis pas sûr qu'il y ait un impact sur la productivité si le processus d'examen est proportionnel pour les raisons suivantes:

  • Il garde les programmeurs honnêtes - si vous savez qu'il sera examiné, les utilisateurs prendront moins de raccourcis
  • Il aide les nouveaux programmeurs à apprendre de programmeurs plus expérimentés
  • Il permet de transférer des connaissances spécifiques au domaine
  • La révision est une autre porte où les bogues et les problèmes potentiels peuvent être trouvés et corrigés

En ne permettant pas à tous les programmeurs d'utiliser le contrôle de code source, ils perdent la possibilité de suivre les modifications, d'annuler les erreurs et de voir un historique raisonnable des modifications. Je ne suis pas sûr que vous souhaitiez que seuls vos "meilleurs" programmeurs puissent se connecter à git.

Cela dit, je pense qu'il est raisonnable que vous ayez quelqu'un qui est en charge de certaines branches clés, comme une branche de publication. Dans ce cas, j'imagine que tout le monde peut utiliser le référentiel git, mais seulement certaines personnes fusionnent dans la branche release. Je ne suis pas sûr qu'il existe un moyen de faire respecter cela dans git, mais il devrait être possible de le faire par processus et de simplement vérifier que personne d'autre ne s'y est inscrit.

La fusion dans la branche de publication pourrait être effectuée par les "meilleurs" programmeurs, ou plus probablement par des personnes compétentes après un examen suffisant.

Steve
la source
1
-1: Cela permet aux programmeurs d'être honnêtes - si vous savez qu'il sera révisé, les utilisateurs prendront moins de raccourcis. - Hmm ... Je serais préoccupé par l'introduction de l'aléa moral. Autrement dit, les développeurs peuvent devenir paresseux ou bâclés car ils savent qu'un développeur plus expérimenté sera toujours responsable de leur code sous la forme d'une révision de code.
Jim G.
1
Le réviseur n'assume aucune responsabilité pour le code, mais donne plutôt des conseils et des instructions sur les problèmes avec le code. Le développeur d'origine doit résoudre les problèmes et est toujours responsable du code.
Steve
-1

les bons programmeurs quittent-ils si une partie importante de leur travail consiste à revoir le code des autres?

S'ils n'apprécient pas le travail et sont forcés de faire cette activité, alors OUI. Cela risque fort de se produire. Car trouver un prochain travail intéressant pour un bon développeur n'est pas un gros défi de nos jours.

Vos meilleurs programmeurs devraient-ils vérifier le code de tout le monde dans le contrôle de code source?

Absolument pas. C'est assurément une perte de temps, à l'exception d'une logique critique qui doit être à l'état solide .

Cependant, les développeurs juniors ou expérimentés devraient probablement être en période de probation pour une qualité de code , juste pour être sûr et s'assurer que leur code suit les directives de développement de l'équipe, au moins pendant quelques semaines avant d'obtenir le privilège de s'engager.

EL Yusubov
la source