Une révision de code qui utilise uniquement des commentaires de code est-elle une bonne idée?

Conditions préalables

• L'équipe utilise DVCS
• IDE prend en charge l'analyse des commentaires (comme TODO et etc.)
• Des outils comme CodeCollaborator sont chers pour le budget
• Des outils comme gerrit sont trop complexes à installer ou inutilisables

Workflow

• L'auteur publie quelque part sur la branche de la fonction de repo centrale
• Le réviseur le récupère et commence la révision

• En cas de question / problème, le réviseur crée un commentaire avec une étiquette spéciale, comme "REV". Une telle étiquette NE DOIT PAS être dans le code de production - uniquement au stade de l'examen:

  $somevar = 123;
  // REV Why do echo this here?
  echo $somevar;
  

• Lorsque le réviseur a terminé de poster des commentaires - il commet simplement avec un message stupide "commentaires" et repousse

• L'auteur récupère la branche de fonctionnalité et répond aux commentaires de manière similaire ou améliore le code et le repousse
• Quand les commentaires "REV" ont disparu, nous pouvons penser que cet examen s'est terminé avec succès.
• L'auteur rebase de manière interactive la branche de la fonctionnalité, la supprime pour supprimer les validations de "commentaire" et est maintenant prêt à fusionner la fonctionnalité pour développer ou effectuer toute action qui pourrait normalement être effectuée après un examen interne réussi.

Prise en charge IDE

Je sais que les balises de commentaires personnalisées sont possibles dans eclipse et netbeans. Bien sûr, il devrait également être dans la famille blablaStorm.

Des questions

1. Pensez-vous que cette méthodologie est viable?
2. Savez-vous quelque chose de similaire?
3. Que peut-on y améliorer?

L'idée est en fait très sympa. Contrairement aux workflows courants, vous conservez la révision directement dans le code, donc techniquement, vous n'avez besoin que d'un éditeur de texte pour utiliser ce workflow. Le support dans l'IDE est également agréable, en particulier la possibilité d'afficher la liste des avis en bas.

Il y a encore quelques inconvénients:

• Cela fonctionne bien pour les très petites équipes, mais les équipes plus grandes auront besoin de suivre ce qui a été examiné, quand, par qui et avec quel résultat. Bien que vous ayez réellement ce type de suivi (le contrôle de version permet de tout savoir), il est extrêmement difficile à utiliser et à rechercher, et nécessitera souvent une recherche manuelle ou semi-manuelle à travers les révisions.

• Je ne crois pas que le réviseur ait suffisamment de commentaires de la part du critique pour savoir comment les points examinés ont été réellement appliqués .

  Imaginez la situation suivante. Alice passe en revue pour la première fois le code d'Eric. Elle remarque qu'Eric, un jeune développeur, a utilisé la syntaxe qui n'est pas la plus descriptive du langage de programmation réellement utilisé.

  Alice suggère une syntaxe alternative et soumet le code à Eric. Il réécrit le code en utilisant cette syntaxe alternative qu'il croit comprendre correctement et supprime le // BLAcommentaire correspondant .

  La semaine prochaine, elle reçoit le code de la deuxième revue. Serait-elle capable de se rappeler qu'elle a fait cette remarque lors de son premier examen, afin de voir comment Eric l'a résolu?

  Dans un processus d'examen plus formel, Alice pouvait immédiatement voir qu'elle avait fait une remarque et voir la différence du code pertinent, afin de remarquer qu'Eric avait mal compris la syntaxe dont elle lui avait parlé.

• Les gens sont toujours des gens. Je suis presque sûr que certains de ces commentaires finiront dans le code de production, et certains resteront comme des ordures tout en étant complètement obsolètes .

  Bien sûr, le même problème existe avec tout autre commentaire; par exemple, il y a beaucoup de commentaires TODO en production (y compris le plus utile: "TODO: commentez le code ci-dessous."), et beaucoup de commentaires qui n'ont pas été mis à jour lorsque le code correspondant était.

  Par exemple, l'auteur original du code ou le réviseur peuvent quitter, et le nouveau développeur ne comprendrait pas ce que dit la critique, donc le commentaire restera pour toujours, en attendant que quelqu'un soit trop courageux pour l'effacer ou pour réellement comprendre ce que ça dit.

• Cela ne remplace pas un examen en face à face (mais le même problème s'applique également à tout autre examen plus formel qui n'est pas effectué en face à face).

  Surtout, si la révision d'origine nécessite des explications, le réviseur et le révisé entament une sorte de discussion . Non seulement vous vous retrouverez avec de grands commentaires BLA, mais ces discussions pollueront également le journal du contrôle de version .

• Vous pouvez également rencontrer des problèmes mineurs avec la syntaxe (qui existe également pour les commentaires TODO). Par exemple, que se passe-t-il si un long commentaire "// BLA" apparaît sur plusieurs lignes et que quelqu'un décide de l'écrire de cette façon:

  // BLA This is a very long comment which is way beyond 80 characters, so it actually
  // fills more then one line. Since the next lines start with slashes, but not "BLA"
  // keyword, the IDE may not be able to show those lines, and display only the first one.
  

• Et enfin comme note mineure et très personnelle: ne choisissez pas "BLA" comme mot-clé. Cela semble moche. ;)

Je voudrais compléter les commentaires dans le code avec un document d'accompagnement. Cela résumerait les résultats et continuerait après la suppression des commentaires. Les avantages de ceci seraient:

• compacité. Si la personne ne vérifie pas systématiquement qu'un pointeur transmis n'est pas nul (ou une autre erreur courante pour les débutants dans la langue que vous utilisez), le réviseur peut laisser des dizaines de commentaires REV à cet effet, et dans le document peut dire " J'ai trouvé 37 endroits où les pointeurs n'ont pas été vérifiés en premier "sans les énumérer tous
• lieu de louange. Un commentaire comme REV this is a nice designsemble un peu étrange, mais mes révisions de code incluent souvent l'approbation ainsi que des corrections
• l'interactivité. Votre exemple de commentaire est "pourquoi avez-vous fait cela?" et c'est très typique. Une approche basée uniquement sur les commentaires ne prend pas en charge une conversation. La personne peut modifier son code et supprimer le commentaire, ou supprimer le commentaire. Mais "pourquoi avez-vous fait ça?" et "veuillez changer cela, c'est faux" ne sont pas la même chose.
• tenir un registre. Un réviseur ultérieur pourra vérifier si le code a bien été modifié ou si les commentaires ont juste été supprimés. Ils peuvent également vérifier que les commentaires ont été "pris en compte" et que le développeur ne commet plus les mêmes erreurs lors d'un examen ultérieur.

J'utiliserais également un élément de travail pour effectuer l'examen et répondre à l'examen, et associer les enregistrements à celui-ci. Cela facilite la recherche des commentaires dans un ancien ensemble de modifications et la façon dont chacun a été traité dans un autre ensemble de modifications.

D'autres ont évoqué les limites de cette approche. Vous avez mentionné que les outils comme gerrit étaient difficiles à installer - je vous suggère de jeter un œil à phabricator (http://www.phabricator.com). Il s'agit du système de révision de code que Facebook utilise depuis des années et a récemment été ouvert. Il n'est pas difficile à installer, a d'excellents flux de travail et résout tous les problèmes mentionnés dans les autres commentaires.