Le développeur senior de notre boutique insiste sur le fait que chaque fois que le code est modifié, le programmeur responsable doit ajouter un commentaire en ligne indiquant ce qu'il a fait. Ces commentaires ressemblent généralement à// YYYY-MM-DD <User ID> Added this IF block per bug 1234.

Nous utilisons TFS pour le contrôle des révisions, et il me semble que les commentaires de ce type sont beaucoup plus appropriés comme notes d'enregistrement plutôt que comme bruit en ligne. TFS vous permet même d'associer un check-in à un ou plusieurs bugs. Certains de nos fichiers de classe plus anciens et souvent modifiés semblent avoir un rapport commentaire / LOC proche de 1: 1. À mes yeux, ces commentaires rendent le code plus difficile à lire et ajoutent une valeur nulle.

Est-ce une pratique standard (ou du moins courante) dans d'autres magasins?

Je considère généralement ces commentaires comme une mauvaise pratique et je pense que ce type d'informations appartient aux journaux de validation SCM. Cela rend le code plus difficile à lire dans la plupart des cas.

Cependant , je fais encore souvent quelque chose comme ça pour des types spécifiques de modifications.

Cas 1 - Tâches

Si vous utilisez un IDE comme Eclipse, Netbeans, Visual Studio (ou si vous avez un moyen de faire des recherches de texte sur votre base de code avec autre chose), votre équipe utilise peut-être des "balises de commentaire" ou des "balises de tâche" spécifiques. Dans ce cas, cela peut être utile.

Je voudrais de temps en temps, lors de l'examen du code, ajouter quelque chose comme ceci:

// TOREVIEW: [2010-12-09 haylem] marking this for review because blablabla

ou:

// FIXME: [2010-12-09 haylem] marking this for review because blablabla

J'utilise différentes balises de tâche personnalisées que je peux voir dans Eclipse dans la vue des tâches pour cela, car avoir quelque chose dans les journaux de validation est une bonne chose mais pas suffisant lorsque vous avez un cadre vous demandant lors d'une réunion de révision pourquoi le bugfix XY a été complètement oublié et a glissé à travers. Donc, sur des questions urgentes ou des morceaux de code vraiment douteux, cela sert de rappel supplémentaire (mais généralement je garderai le commentaire court et vérifierai les journaux de validation parce que c'est à ça que sert le rappel, donc je n'encombre pas trop le code beaucoup).

Cas 2 - Correctifs de libs tiers

Si mon produit doit empaqueter un morceau de code tiers en tant que source (ou bibliothèque, mais reconstruit à partir de la source) parce qu'il devait être corrigé pour une raison quelconque, nous documentons le correctif dans un document séparé où nous listons ces "mises en garde" pour référence future, et le code source contiendra généralement un commentaire similaire à:

// [PATCH_START:product_name]
//  ... real code here ...
// [PATCH_END:product_name]

Cas 3 - Corrections non évidentes

Celui-ci est un peu plus controversé et plus proche de ce que demande votre développeur senior.

Dans le produit sur lequel je travaille en ce moment, nous avons parfois (certainement pas une chose courante) un commentaire comme:

// BUGFIX: [2010-12-09 haylem] fix for BUG_ID-XYZ

Nous ne le faisons que si le correctif n'est pas évident et que le code se lit anormalement. Cela peut être le cas pour les bizarreries du navigateur par exemple, ou les correctifs CSS obscurs que vous devez implémenter uniquement parce qu'il y a un bogue de document dans un produit. Donc, en général, nous le lions à notre référentiel de problèmes interne, qui contiendra ensuite le raisonnement détaillé derrière le correctif et des pointeurs vers la documentation du bogue du produit externe (par exemple, un avis de sécurité pour un défaut bien connu d'Internet Explorer 6, ou quelque chose comme ca).

Mais comme mentionné, c'est assez rare. Et grâce aux balises de tâche, nous pouvons les parcourir régulièrement et vérifier si ces correctifs étranges ont toujours un sens ou peuvent être supprimés (par exemple, si nous avons abandonné la prise en charge du produit buggy à l'origine du bogue).

Ceci juste dans: Un exemple réel

Dans certains cas, c'est mieux que rien :)

Je suis juste tombé sur une énorme classe de calcul statistique dans ma base de code, où le commentaire d'en-tête était sous la forme d'un journal des modifications avec le yadda yadda habituel: relecteur, date, ID de bogue.

Au début, j'ai pensé à la mise au rebut, mais j'ai remarqué que les ID de bogue ne correspondaient pas seulement à la convention de notre outil de suivi des problèmes actuel, mais ne correspondaient pas non plus à celui du système de suivi utilisé avant de rejoindre la société. J'ai donc essayé de lire le code et de comprendre ce que la classe faisait (pas d'être un statisticien) et j'ai également essayé de déterrer ces rapports de défauts. En l'occurrence, ils étaient assez importants et auraient raccourci la vie du gars suivant pour éditer le fichier sans le savoir assez horrible, car il traitait de problèmes de précision mineurs et de cas spéciaux basés sur des exigences très spécifiques émises par le client d'origine à l'époque. . En bout de ligne, si ceux-ci n'avaient pas été là-dedans, je ne l'aurais pas su. S'ils n'avaient pas été là-bas ET que j'avais eu une meilleure compréhension de la classe,

Parfois, il est difficile de garder une trace d'exigences très anciennes comme celles-ci. En fin de compte, ce que j'ai fait était toujours de supprimer l'en-tête, mais après avoir glissé dans un commentaire de bloc avant chaque fonction incriminante décrivant pourquoi ces calculs "étranges" car ce sont des demandes spécifiques.

Donc, dans ce cas, je les considérais toujours comme une mauvaise pratique, mais mon garçon était heureux que le développeur d'origine les ait au moins insérés! Il aurait mieux valu commenter le code clairement à la place, mais je suppose que c'était mieux que rien.

Nous utilisons cette pratique pour les fichiers qui ne sont pas sous contrôle de version. Nous avons des programmes RPG qui fonctionnent sur un ordinateur central, et il s'est avéré difficile de faire bien plus que de les sauvegarder.

Pour le code source versionné, nous utilisons les notes d'enregistrement. Je pense que c'est là qu'ils appartiennent, pas encombrer le code. Ce sont des métadonnées, après tout.

Nous avions cette pratique il y a longtemps dans un magasin SW où notre responsable insistait pour qu'il souhaite pouvoir lire les fichiers de code sur papier et suivre leur historique de révision. Inutile de dire que je ne me souviens d'aucune occasion concrète où il a regardé l'impression d'un fichier source: - /

Heureusement, mes managers ont depuis lors été plus informés sur ce que les systèmes de contrôle de version peuvent faire (je dois ajouter que nous avons utilisé SourceSafe dans ce premier magasin). Je n'ajoute donc pas de métadonnées de version dans le code.

Ce n'est généralement pas nécessaire si votre SCM et votre IDE vous permettent d'utiliser la fonction "afficher l'annotation" (appelée dans Eclipse de toute façon), vous pouvez facilement voir quel (s) commit (s) a changé un morceau de code, et les commentaires de commit devraient dire qui et pourquoi.

La seule fois où je mettrais un commentaire comme celui-ci dans le code, c'est si c'est un morceau de code particulièrement étrange qui peut causer de la confusion à quelqu'un à l'avenir, je commenterai brièvement avec le numéro de bogue afin qu'ils puissent aller au rapport de bogue et lire en détail à ce sujet.

De toute évidence, de tels commentaires sont presque garantis d'être incorrects au fil du temps; si vous modifiez une ligne deux fois, ajoutez-vous deux commentaires? Où vont-ils? Un meilleur processus consiste donc à compter sur vos outils.

Ceci est un signe que, pour une raison quelconque, le développeur senior ne peut pas compter sur le nouveau processus pour suivre les modifications et connecter les modifications au système de suivi des problèmes. Vous devez remédier à cet inconfort pour résoudre le conflit.

Vous devez comprendre pourquoi il ne peut pas compter dessus. S'agit-il de vieilles habitudes? Une mauvaise expérience avec le système SCM? Un format de présentation qui ne fonctionne pas pour lui? Peut-être qu'il ne connaît même pas les outils tels que `` git blame '' et la vue chronologique de Perforce (qui le montre essentiellement, bien qu'il ne puisse pas montrer quel problème a déclenché le changement).

Sans comprendre la raison de cette exigence, vous ne pourrez pas le convaincre.

Je travaille sur un produit Windows de plus de 20 ans. Nous avons une pratique similaire qui est en place depuis longtemps. Je ne peux pas compter le nombre de fois où cette pratique a sauvé mon bacon.

Je suis d'accord que certaines choses sont redondantes. Auparavant, les développeurs utilisaient cette pratique pour commenter le code. Quand je revois cela, je leur dis d'aller de l'avant et de supprimer le code, et le commentaire n'est pas nécessaire.

Mais, souvent, vous pouvez regarder du code qui a été autour d'une décennie et modifié par 20 développeurs mais jamais refactorisé. Tout le monde a depuis longtemps oublié tous les détails des exigences qui étaient dans le code d'origine, sans parler de toutes les modifications, et il n'y a pas de documentation fiable.

Un commentaire avec un numéro de défaut me donne un endroit où aller pour rechercher l'origine du code.

Donc, oui, le système SCM fait beaucoup, mais pas tout. Traitez-les comme vous le feriez pour tout autre commentaire et ajoutez-les chaque fois qu'une modification importante est apportée. Le développeur qui regarde votre code depuis plus de 5 ans vous en remerciera.

Mentionner chaque changement dans les commentaires est inutile si vous avez un VCS. Mentionner un bogue spécifique ou une autre note importante liée à une ligne spécifique est utile. Un changement peut inclure plusieurs lignes modifiées, toutes sous le même message de validation.

Pas que je sache.

Je fais des éclaboussures de TODO dans mon code au fur et à mesure, et le but est de les faire supprimer d'ici le temps de révision.