Comment amener les développeurs à effectuer des révisions de code en temps opportun

12

La société pour laquelle je travaille exige que tout le code soit examiné par d'autres développeurs avant d'être validé. Les membres de mon équipe sont souvent frustrés car les autres développeurs sont trop occupés à coder pour faire une revue, surtout si elle est très longue. Comment incitez-vous d'autres développeurs à effectuer des révisions de code en temps opportun?

(Nous utilisons git-svn afin que nous puissions continuer à coder en attendant une révision. Cependant, je trouve toujours frustrant de devoir attendre longtemps avant de pouvoir valider mon code.)

titaniumdecoy
la source

Réponses:

12

Regardez comment Facebook le fait avec leur propre application, appelée phabricator: http://phabricator.org/

Ils s'engagent essentiellement pour chaque problème, et pour chaque problème, le code est affiché, qui doit être examiné par quelqu'un. Le code ne va pas dans leur référentiel principal jusqu'à ce que le réviseur ait dit que c'est ok pour le faire.

Je suppose que cela le rend plus amusant.

De plus, un code devrait peut-être être attribué à deux personnes: une qui le fait et une qui le révise.

Bien que vos coéquipiers ne croient peut-être pas en cette critique.

Personnellement, en l'absence de réviseurs, j'ai utilisé des tests unitaires pour les fonctions de niveau inférieur et "le test du concierge" pour tout le reste: le test du concierge est appelé ainsi, car même le concierge devrait être capable de comprendre votre code.

J'ai généralement supprimé certaines parties mineures, comme les crochets de portée de bloc / fonction, les notations de visibilité, parfois même les types, et je l'ai montré aux gestionnaires, aux experts de domaine, aux partenaires, à la personne qui a demandé le code: "est-ce ce que vous voulez?"

De plus, y aller personnellement et ne pas partir tant que la révision n'est pas terminée aide.

Ou, au cas où vous ne seriez pas d'accord avec l'équipe, ou qu'ils ne vous conviendraient pas, vous savez, "si vous pouvez 'changer l'entreprise, changer d'entreprise" ...

Aadaam
la source
9

Je vais baser cela sur quelques hypothèses:

  1. Tout le monde semble vouloir écrire du code (sinon, il y a des gens qui doivent y aller.).
  2. Tout le monde veut que son propre code soit archivé.

Autorisez uniquement ceux qui ont terminé leurs évaluations à faire enregistrer leur code.

Peut-être qu'un certain bloc de temps peut être consacré aux revues de code dans l'espoir d'éviter le problème de l'interruption.

L'objectif doit être de vérifier le code de qualité. Vous ne voulez pas punir / forcer les critiques au point que tout le monde lui donne simplement une approbation "tampon en caoutchouc".

Si vous avez différents niveaux (jr., Sr. Etc.), la promotion et le maintien d'un titre doivent dépendre de votre travail.

JeffO
la source
1

Chez deux ou trois employeurs précédents, nous avons alterné ceux qui étaient quotidiennement en révision de code. Habituellement, 3 personnes partageaient une journée CR et chaque CR devait être approuvé par deux des examinateurs. Cela a fait en sorte que lorsque c'était votre journée, vous saviez que CR était attendu de vous, quels que soient vos autres projets. Donc, tous les cinq jours environ, c'était votre tour et vous pouviez ajuster vos tâches de développement en conséquence.

Actuellement, un chef d'équipe fait un CR rapide sur le code de son équipe. Selon le domaine de l'application qui a été mis à jour, une fois le CR terminé, il peut être renvoyé à l'équipe d'examen global, où nous vérifions les éléments qui ont un impact global sur les applications, au lieu des éléments qui sont liés à une seule fonctionnalité. Quand il y a un grand examen à faire, nous le séparons généralement entre quelques personnes afin que personne ne doive faire face aux changements dans un nombre ridicule de fichiers.

Cela dit, nous ne révisons que le code qui a été validé pour la branche / variante de développement actuelle. Le code doit passer un examen du code, un examen global, un examen de la base de données et un examen de l'interface utilisateur (chacun selon les besoins) avant de pouvoir être promu dans le prochain environnement (par exemple Alpha).

Enfin, nous convenons d'un SLA sur la rapidité avec laquelle les avis sont retournés. Presque rien n'est en attente depuis plus de 48 heures et la plupart des examens sont effectués en moins de 24 heures.

Adrian J. Moreno
la source
1

La société pour laquelle je travaille exige que tout le code soit examiné par d'autres développeurs avant d'être validé. Les membres de mon équipe sont souvent frustrés ...

À moins que vous n'utilisiez des logiciels critiques ou des correctifs critiques pour la production de code candidat, il n'y a aucune raison impérieuse de s'en tenir rigoureusement à un type particulier de révision de code.

  • L'idée sous-jacente aux exigences de votre entreprise semble raisonnable à première vue (code révisé à 100%), mais les moyens qu'ils ont décidé d'utiliser sont contre-productifs - car, comme vous le signalez, cela conduit les développeurs à la frustration.

En marchant dans vos chaussures, je voudrais tout d'abord signaler à la direction que les revues de code post-commit sont considérées comme aussi respectables que les revues pré-commit . Ces termes sont consultables sur le Web - si nécessaire, trouvez des références pour les sauvegarder. On n'a pas nécessairement besoin de révisions avant validation pour obtenir un code révisé à 100%.

Sur la base de ce qui précède, je leur suggérerais ensuite de laisser tomber l' attitude de microgestion et de laisser les développeurs essayer la manière qui leur semble la plus pratique. Il est préférable de laisser les programmeurs avant ou après la validation des choix. Si l'entreprise sait mieux que les programmeurs, pourquoi n'écrivent-ils pas eux-mêmes le code?

moucheron
la source
1
"Si l'entreprise sait mieux que les programmeurs, pourquoi n'écrivent-ils pas eux-mêmes le code?": Très bon commentaire! Mais j'espère que les responsables du développement sont eux-mêmes d'anciens développeurs.
Giorgio
3
D'après mon expérience, la post-validation nuit énormément à la qualité de votre code. Les programmeurs sont paresseux, et ils sentiront qu'ils ont terminé si cela peut être commis: "Ouais, eh bien, ce n'est pas parfait, mais à qui le f.ck se soucie, qu'est-ce qui est parfait dans la vie? Il fait le travail, n'est-ce pas? " la seule bonne réponse est NON, et peut-être que les gestionnaires ont une image des programmeurs plus réaliste qu'eux-mêmes, c'est pourquoi ils nécessitent des révisions de code pré-validées (ou du moins pré-fusionnées).
Aadaam
@Aadaam, votre expérience diffère définitivement de la mienne - non seulement en ce qui concerne les post-commits, mais aussi (et surtout) avec une partie de "Les programmeurs sont paresseux ...". mes équipes; cela ne conduisait pas les managers avec lesquels je travaillais à des idées insensées sur le type de révision à forcer
gnat
Eh bien, j'ai travaillé dans l'externalisation. Dans l'externalisation, la plupart des programmeurs ne sont pas là parce que la programmation est amusante, ils le sont parce que la programmation a le meilleur rapport travail / salaire, avec des taux beaucoup plus élevés que toute autre chose ... ils détestent ça comme n'importe quel autre travail .. et ils essayez de tout faire pour "optimiser" davantage ce rapport, si vous voyez ce que je veux dire ...
Aadaam
1

Vous avez un certain nombre de problèmes à résoudre - vous devez gagner les cœurs et les esprits et vous devez vous assurer que du temps est disponible pour les révisions de code.

La deuxième partie est probablement la plus simple - vous convenez (collectivement et cela doit inclure la direction) que la première chose qu'un développeur fait chaque matin est ses révisions de code - c'est simple, compréhensible, efficace et vous donne un bon bâton clair pour battre les gens avec s'ils ne se conforment pas. Mieux, vous n'interrompez rien, vous ne leur demandez pas d'arrêter de travailler sur leur code, vous ne demandez pas aux gens de mettre quelque chose dans leur liste de choses à faire ...

La première partie est le vrai problème - les participants au processus de révision doivent le voir comme ayant de la valeur sinon ils ne feront jamais de révision de code (qui est perçu comme n'ayant pas de valeur) quand ils pourraient écrire du code ou corriger des bogues (ce qui est sûrement plus important ...?).

Si vous pouvez combiner les deux - tout d'abord en vous assurant que tout le monde croit (ou comprend) qu'il y a de la valeur dans les revues de code - au plus basique, cela devrait signifier moins de bogues, ce qui signifie plus de nouveau code qui est généralement plus amusant - et ensuite en organisant les choses afin qu'il y ait un espace clair dans le calendrier pour les révisions de code à faire, alors j'espère que de bonnes choses se produiront ... cela fera partie de la culture.

Une fois que cela fait partie de la culture, il n'est peut-être plus nécessaire de dire "la première chose tous les jours" - mais cela étant dit, je pense que cela correspond bien au modèle, on veut probablement qu'un développeur travaille.

Murph
la source
Vous ne pouvez pas vraiment accepter cette règle de la «première chose tous les jours» en premier lieu. Si quelqu'un trouve un bogue qui coûte à l'entreprise X dollars par heure (ou augmente le risque de manquer une échéance importante de X points de pourcentage par heure), et qu'ils le font cinq minutes avant votre entrée, la révision du code n'est pas votre priorité. Fondamentalement, le problème est le conflit entre la volonté de fixer des règles simples et le fait que les priorités ne sont pas toujours simples. Le risque est que tout le monde accepte de tout cœur la règle et, dans les 24 heures, trouve une raison pour laquelle aujourd'hui est une exception à la règle.
Steve Jessop
Et la solution est délicate, mais IME vous devez trouver suffisamment "d'espace" pour introduire une nouvelle pratique de travail qui prend du temps mais qui en vaut la peine. Cela nécessite une clairvoyance pour identifier le temps libre, une volonté de laisser les délais s'écouler comme le prix de l'introduction d'un changement valable, ou les deux. TANSTAAFL. Comme vous dites, une fois que tout le monde est installé dans le modèle, il peut faire des exceptions. J'espère qu'ils le feront sur la base d'une appréciation appropriée de la valeur du schéma général.
Steve Jessop
Je dis "laissez les délais glisser", j'aurais dû dire "déplacer les délais". "slip" implique de les déplacer après leur validation, c'est-à-dire d'échouer, mais il n'est pas nécessaire qu'il en soit ainsi. Vous pouvez plutôt planifier une période de productivité légèrement réduite en raison de la nouvelle règle inflexible (et des inefficacités inévitables causées par les gens qui essaient de suivre tout nouveau processus - si vous faites la révision du code en premier, alors vous raterez la mêlée du matin se réunir les jours où la révision du code prend trop de temps, ou quels que soient les inconvénients uniques que votre organisation peut jeter dans le mélange). Si c'est une bonne règle, vous arriverez bientôt au-dessus de votre point de départ.
Steve Jessop
@SteveJessop bien sûr, vous pouvez vraiment être d'accord. Et bien sûr, il va y avoir des exceptions (je pense que l'observation de la mêlée est idiote - d'autant plus que la réponse est évidente (-:). Je pense que la clé est qu'il n'y a pas de "solution universelle" a proposé quelque chose de simple et de facile à comprendre et qui est relativement difficile à bousculer dans son emploi du temps (encore une fois en fonction de l'environnement)
Murph
1

Dans la plupart des entreprises pour lesquelles j'ai travaillé, vous avez 3 jours pour effectuer une évaluation. Il n'est pas acceptable de ne pas faire les évaluations. Cela fait partie de votre travail. Si vous ne faites pas de critiques décentes à temps, cela affecte votre évaluation des performances. Et oui, les revues semblent toujours se produire aux moments les plus inopportuns. Dommage, apprenez à inclure le temps de révision dans vos estimations. Quoi qu'il en soit, si la direction croit vraiment que les examens sont importants (c.-à-d. Qu'elle exige que tout le code soit examiné), elle préconisera une politique similaire. De plus, si les gens ne terminent pas l'examen dans le temps imparti, cela va de pair avec leur acceptation du matériel.

Tremper
la source
0

Pensez à utiliser un outil comme Review Board . C'est très utile, surtout pour les longues critiques.

Vous pouvez télécharger vos différences et attendre qu'un réviseur ait terminé sa révision. Si vous avez des critiques ouvertes qui vous empêchent de poursuivre votre travail, vous pouvez le signaler lors de vos réunions quotidiennes (votre équipe souhaite que les nouvelles fonctionnalités soient enregistrées afin qu'elles puissent être testées dès que possible, n'est-ce pas?).

Giorgio
la source
0

Quelques points à ajouter qui ne figurent pas dans les autres réponses.

Le code à réviser doit être archivé

  • afin que vous examiniez une version stable.
  • Il peut être sur la branche principale de développement si une version est suffisamment éloignée
  • Il peut être sur branche s'il y a une bonne raison de ne pas polluer

Les tâches de blocage sont prioritaires, les révisions de code doivent donc avoir priorité sur les autres travaux (mais en essayant de ne pas interrompre votre flux). En tant que développeur, vous devriez souhaiter que d'autres révisent votre code (car vous visez à l'améliorer). Dans cette connaissance, vous devez effectuer rapidement des révisions pour les autres.

Les questions les plus difficiles sont de savoir quand et comment bien réviser le code.

Une règle qui a fonctionné pour nous sur le moment est que le code partagé doit être révisé car il a un impact plus large alors que dans le code pour une seule application (surtout étant donné que nous utilisons le développement piloté par les tests), c'est moins important.

Bruce Adams
la source