Réconcilier la règle du scoutisme et le refactoring opportuniste avec les révisions de code

Je suis un fervent partisan de la règle du scoutisme :

Vérifiez toujours un module plus propre que lorsque vous l'avez vérifié. "Quel que soit l'auteur d'origine, nous ferions toujours des efforts, aussi petits soient-ils, pour améliorer le module. Quel serait le résultat? Je pense que si tous suivaient cette règle simple, nous verrions la fin de la détérioration incessante de nos systèmes logiciels, mais nos systèmes s’amélioreraient progressivement au fil de leur évolution, et nous verrions également des équipes s’occuper du système dans son ensemble. que de simples personnes prenant soin de leur propre petite petite partie.

Je suis également un fervent partisan de l'idée connexe de la refactorisation opportuniste :

Bien qu'il existe des endroits pour certains efforts de refactorisation programmés, je préfère encourager la refactorisation en tant qu'activité opportuniste, à effectuer quand et où le code doit être nettoyé - par qui que ce soit. Cela signifie qu'à tout moment, quelqu'un verra du code qui n'est pas aussi clair qu'il le devrait, il devrait en profiter pour le corriger immédiatement - ou du moins dans quelques minutes.

Notez en particulier l'extrait suivant de l'article de refactoring:

Je me méfie des pratiques de développement susceptibles de créer des frictions lors de la refactorisation opportuniste ... Mon sentiment est que la plupart des équipes ne font pas assez de refactorisation. Il est donc important de faire attention à tout ce qui décourage les gens. Pour vous aider à résoudre ce problème, soyez conscient de chaque fois que vous vous sentez découragé de procéder à une petite refactorisation, celle-ci ne vous prendra que quelques minutes. Une telle barrière est une odeur qui devrait déclencher une conversation. Alors, prenez note du découragement et soulevez-le avec l'équipe. À tout le moins, cela devrait être discuté lors de votre prochaine rétrospective.

Là où je travaille, il existe une pratique de développement qui provoque de fortes frictions - la révision de code (CR). Chaque fois que je change quelque chose qui n'entre pas dans le cadre de ma "tâche", mes critiques me reprochent de rendre le changement plus difficile à contrôler. Cela est particulièrement vrai en cas de refactorisation, car cela rend difficile la comparaison "ligne par ligne" des diff. Cette approche est la norme ici, ce qui signifie que la refactorisation opportuniste est rarement effectuée et que seule la refactorisation "planifiée" (qui est généralement trop petite, trop tard) a lieu, voire pas du tout.

J'affirme que les avantages en valent la peine, et que 3 réviseurs travailleront un peu plus fort (pour comprendre réellement le code avant et après, plutôt que de regarder l'étendue des lignes qui ont changé - la révision elle-même serait meilleure à cause de cela ) afin que les 100 prochains développeurs qui lisent et gèrent le code en tirent profit. Lorsque je présente cet argument à mes relecteurs, ils me disent qu'ils n'ont aucun problème avec mon refactoring, tant que ce n'est pas dans le même CR. Cependant, je prétends que c'est un mythe:

(1) La plupart du temps, vous réalisez seulement quoi et comment vous voulez procéder à une refactorisation lorsque vous êtes au milieu de votre mission. Comme le dit Martin Fowler:

Au fur et à mesure que vous ajoutez la fonctionnalité, vous réalisez que le code que vous ajoutez contient une certaine duplication avec du code existant. Vous devez donc refactoriser le code existant pour nettoyer les choses ... Vous pouvez obtenir quelque chose de fonctionnel, mais vous devez réaliser mieux si l’interaction avec les classes existantes était modifiée. Profitez de cette occasion pour le faire avant de vous considérer comme fait.

(2) Personne ne vous considérera comme favorable à la publication de "refactoring" de CR que vous n'étiez pas censés faire. Un CR a un certain surcoût et votre responsable ne veut pas que vous perdiez votre temps en refactoring. Lorsque le changement que vous êtes censé apporter est intégré, le problème est minimisé.

La question est exacerbée par Resharper, car chaque nouveau fichier que j’ajoute au changement (et je ne sais pas à l’avance quels fichiers seront modifiés) est généralement semé d’erreurs et de suggestions - la plupart d’entre elles sont parfaitement justes et méritent totalement. fixation.

Le résultat final est que je vois un code horrible, et je le laisse là. Paradoxalement, j’estime que la correction de ce code non seulement n’améliorera pas mon classement, mais le diminuera et me présentera comme le type "flou" qui perd son temps à réparer des choses qui ne s’intéressent pas à la place de son travail. Je me sens mal parce que je méprise vraiment le mauvais code et que je ne supporte pas de le regarder, encore moins de l'appeler avec mes méthodes!

Des idées sur la façon dont je peux remédier à cette situation?

OK, donc il y a maintenant plus de choses ici qu'il n'y a de place pour un commentaire.

tl; dr

Votre intuition sur ce que vous devez faire (refactoring au fur et à mesure) est correcte.

Votre difficulté à mettre en œuvre ceci - étant donné que vous devez contourner un système de révision de code médiocre - découle de votre difficulté à manipuler votre code source et VCS. Plusieurs personnes ont dit que vous pouvez et devez diviser vos modifications (oui, même dans les fichiers) dans plusieurs commits, mais vous semblez avoir du mal à croire cela.

Vous pouvez vraiment faire ça. C'est vraiment ce que nous proposons. Vous devriez vraiment apprendre à tirer le meilleur parti de vos outils d’édition, de manipulation de source et de contrôle de version. Si vous passez du temps à apprendre à bien les utiliser, cela vous simplifiera la vie.

Problèmes de workflow / bureau

Je ferais essentiellement la même recommandation que GlenH7: vous créez deux commits - un avec seulement des refactorisations et (manifestement ou évidemment) aucune modification fonctionnelle, et un avec les modifications fonctionnelles en question.

Toutefois, il peut être utile, si vous rencontrez beaucoup d’erreurs, de choisir une seule catégorie d’erreurs à corriger dans un seul CR. Ensuite, vous avez un commit avec un commentaire comme "code de déduplication", "corrige les erreurs de type X", ou peu importe. Parce que cela crée un seul type de changement, vraisemblablement à plusieurs endroits, il devrait être facile de le réviser. Cela signifie que vous ne pouvez pas réparer toutes les erreurs que vous trouvez, mais que cela peut être moins pénible de passer clandestinement.

Problèmes VCS

Diviser les modifications apportées à votre source de travail en plusieurs commits ne devrait pas être un défi. Vous n'avez pas dit ce que vous utilisez, mais les workflows possibles sont les suivants:

1. si vous utilisez git, vous avez d’excellents outils pour cela

   • vous pouvez utiliser git add -ipour la mise en scène interactive à partir de la ligne de commande
   • vous pouvez utiliser git guiet sélectionner des éléments individuels et des lignes à mettre en scène (il s'agit de l'un des rares outils d'interface utilisateur graphique liés à VCS que je préfère à la ligne de commande, l'autre étant un bon éditeur de fusion à 3 voies)
   • vous pouvez effectuer de nombreux petits commits (modifications individuelles, ou corriger la même classe de bogues à plusieurs endroits), puis les réorganiser, les fusionner ou les scinder avec rebase -i

2. si vous n'utilisez pas git, votre VCS peut encore disposer d'outils pour ce genre de choses, mais je ne peux pas vous conseiller sans savoir quel système vous utilisez.

   Vous avez mentionné que vous utilisiez TFS - qui, à mon avis, est compatible avec git depuis TFS2013. Il peut être intéressant d’utiliser un clone git local du référentiel pour travailler. Si ceci est désactivé ou ne fonctionne pas pour vous, vous pouvez toujours importer la source dans un référentiel git local, y travailler et l’utiliser pour exportez vos commits finaux.

3. vous pouvez le faire manuellement dans n'importe quel VCS si vous avez accès à des outils de base tels que diffet patch. C'est plus douloureux, mais certainement possible. Le flux de travail de base serait:

   1. effectuez toutes vos modifications, testez, etc.
   2. utiliser diffpour créer un fichier de correctif (contextuel ou unifié) avec toutes les modifications depuis le dernier commit
   3. partitionnez en deux fichiers: vous obtiendrez un fichier de correctif contenant des refactorings et un autre avec des modifications fonctionnelles
      • ce n'est pas tout à fait trivial - des outils tels que le mode diff d'emacs peuvent aider
   4. sauvegarder tout
   5. revenir à la dernière validation, utilisez patchpour relire l'un des fichiers du correctif, commettez ces modifications
      • NB si le correctif ne s’applique pas correctement, vous devrez peut-être réparer manuellement les mecs qui ont échoué.
   6. répéter 5 pour le deuxième fichier patch

Vous avez maintenant deux commits, avec vos modifications partitionnées correctement.

Notez qu'il existe probablement des outils pour rendre cette gestion de correctifs plus facile - je ne les utilise pas car git le fait pour moi.

Je vais supposer que les demandes de changement sont volumineuses et formelles dans votre entreprise. Sinon, apportez les modifications (dans la mesure du possible) à de nombreux petits commits (comme vous êtes censé le faire).

Des idées sur la façon dont je peux remédier à cette situation?

Continuer à faire ce que tu fais?

Je veux dire, toutes vos pensées et vos déductions sont tout à fait correctes. Vous devriez réparer les choses que vous voyez. Les gens ne font pas assez de refactorisation planifiée. Et cet avantage pour l’ensemble du groupe est plus important que de déranger quelques-uns.

Ce qui peut aider, c'est d'être moins combatif. Les revues de code ne devraient pas être combatives "Pourquoi avez-vous fait cela?", Elles devraient être collaboratives "Hé les gars, pendant que j'étais ici, j'ai arrangé tout ça!". Travailler (avec votre responsable principal / responsable si possible) pour changer cette culture est difficile, mais assez vital pour créer une équipe de développement performante.

Vous pouvez également travailler (avec votre responsable principal / responsable si possible) pour promouvoir l’importance de ces idées avec vos collègues. Assurez-vous de demander "pourquoi ne vous souciez-vous pas de la qualité?" au lieu de demander "pourquoi faites-vous toujours ces choses inutiles?".

J'ai beaucoup de sympathie pour votre situation, mais peu de suggestions concrètes. Si rien d'autre, je vais peut-être vous convaincre que si mauvaise que soit la situation, cela pourrait être pire. Cela peut TOUJOURS être pire. :-)

Premièrement, je pense que vous avez (au moins) deux problèmes avec votre culture, pas un seul. L'un des problèmes est l'approche de la refactorisation, mais la révision du code semble être un problème distinct. Je vais essayer de séparer mes pensées.

Avis de code

J'étais dans un groupe qui détestait les critiques de code. Le groupe a été formé par la fusion de deux groupes de différentes parties de la société. Je viens du groupe qui effectue des revues de code depuis plusieurs années, avec des résultats généralement bons. La plupart d'entre nous croyions que les critiques de code étaient une bonne utilisation de notre temps. Nous avons fusionné pour former un groupe plus large et, autant que nous puissions en juger, cet "autre" groupe n'avait même jamais entendu parler de révision de code. Maintenant, nous travaillions tous sur "leur" base de code.

Les choses n'allaient pas bien quand nous avons fusionné. Les nouvelles fonctionnalités accusaient un retard de 6 à 12 mois, année après année. L'arriéré de bogues était énorme, en croissance et fatal. La possession de code était forte, en particulier parmi les plus grands "gourous". Les "branches de fonctions" duraient parfois des années et couvraient quelques versions; parfois personne mais un seul développeur a vu le code avant qu'il ne frappe la branche principale. En fait, "branche de fonctionnalité" est trompeuse, car elle suggère que le code se trouvait quelque part dans le référentiel. Le plus souvent, ce n'était que sur le système individuel du développeur.

La direction a convenu que nous devions «faire quelque chose» avant que la qualité ne devienne trop basse. :-) Leur réponse a été Code Reviews. Les revues de code sont devenues un élément officiel du «tu chalt», à précéder chaque enregistrement. L'outil que nous avons utilisé était Review Board.

Comment cela a-t-il fonctionné dans la culture que j'ai décrite? Mieux que rien, mais cela a été douloureux et il a fallu plus d'un an pour atteindre un niveau de conformité minimal. Certaines choses que nous avons observées:

1. L'outil que vous utilisez a tendance à cibler les révisions de code de certaines manières. Cela peut être un problème. Review Board vous offre de jolies différences colorées ligne à ligne et vous permet de joindre des commentaires à une ligne. Cela fait que la plupart des développeurs ignorent toutes les lignes qui n'ont pas changé. C'est bien pour les petits changements isolés. Ce n'est pas si bon pour les gros changements, les gros morceaux de nouveau code, ou le code qui contient 500 instances d'une fonction renommée mélangée à 10 lignes de nouvelles fonctionnalités.
2. Même si nous étions dans une vieille base de code malade qui n'avait pour la plupart jamais été révisée auparavant, il devenait "impoli" de laisser un critique commenter tout ce qui n'était PAS une ligne de changement, même pour signaler un bug évident. "Ne me dérange pas, c'est un enregistrement important et je n'ai pas le temps de corriger les bugs."
3. Magasinez pour un critique "facile". Certaines personnes vont regarder une révision de 10 fichiers avec 2000 lignes modifiées pendant 3 minutes et cliquer sur "Expédier!". Tout le monde apprend rapidement qui sont ces personnes. Si vous ne voulez vraiment pas que votre code soit revu en premier lieu, envoyez-le à un relecteur "facile". Votre enregistrement ne sera pas ralenti. Vous pouvez retourner la faveur en devenant un relecteur "facile" pour son code.
4. Si vous détestez les critiques de code, ignorez simplement les courriels que vous recevez de Review Board. Ignorez les suivis des membres de votre équipe. Pendant des semaines. Jusqu'à ce que vous ayez 3 douzaines de critiques ouvertes dans votre file d'attente et que votre nom apparaisse à plusieurs reprises lors de réunions de groupe. Devenez ensuite un critique "facile" et effacez toutes vos critiques avant le déjeuner.
5. Évitez d’envoyer votre code à un critique "dur". (Le type de développeur qui poserait une question comme celle-ci ou répondrait à cette question.) Tout le monde apprend rapidement qui sont les critiques "dures", tout comme ils apprennent les plus faciles. Si les critiques de code sont une perte de temps (™), alors lire des commentaires détaillés sur le code que vous OWN constitue à la fois une perte de temps (™) et une insulte.
6. Lorsque les critiques de code sont pénibles, les utilisateurs les repoussent et continuent d'écrire plus de code. Vous recevez moins de critiques de code, mais chacune est grosse. Vous avez besoin de plus de révisions de code plus petites, ce qui signifie que l'équipe doit trouver le moyen de les rendre aussi simples que possible.

Je pensais écrire quelques paragraphes sur Code Code, mais il s’est avéré que j’écrivais surtout sur la culture. Je suppose que cela se résume à ceci: les critiques de code peuvent être bonnes pour un projet, mais une équipe n’obtient que les avantages qu’elle mérite.

Refactoring

Mon groupe a-t-il méprisé le refactoring encore plus qu'il ne détestait les critiques de code? Bien sûr! Pour toutes les raisons évidentes qui ont déjà été mentionnées. Vous perdez mon temps avec une révision de code épineuse, et vous n’ajoutez même pas une fonctionnalité ou ne corrigez pas un bogue!

Mais le code avait toujours besoin de refactoring. La façon de procéder?

1. Ne mélangez jamais un changement de refactoring avec un changement fonctionnel. Un certain nombre de personnes ont mentionné ce point. Si les révisions de code sont un point de friction, n'augmentez pas la friction. C'est plus de travail, mais vous devriez prévoir un examen et un enregistrement séparés.
2. Commencer petit. Très petit. Encore plus petit que ça. Utilisez de très petits refactorisations pour apprendre progressivement aux gens que la refactorisation et les révisions de code ne sont pas un pur mal. Si vous pouvez vous glisser dans une minuscule refactorisation par semaine sans trop de douleur, dans quelques mois, vous pourrez peut-être vous en sortir avec deux par semaine. Et ils peuvent être un peu plus gros. Mieux que rien.
3. Nous n’avions essentiellement PAS de tests unitaires. La refactorisation est donc interdite, non? Pas nécessairement. Pour certaines modifications, le compilateur est votre test unitaire. Concentrez-vous sur les refactorisations que vous pouvez tester. Évitez les changements s'ils ne peuvent pas être testés. Peut-être passer du temps à écrire des tests unitaires à la place.
4. Certains développeurs ont peur de la refactorisation car ils ont peur de TOUS les changements de code. Il m'a fallu beaucoup de temps pour reconnaître ce type. Ecrivez un morceau de code, manipulez-le jusqu'à ce qu'il "fonctionne", puis ne voulez JAMAIS le changer. Ils ne comprennent pas nécessairement POURQUOI cela fonctionne. Le changement est risqué. Ils ne se font pas confiance pour faire AUCUN changement et ils ne vous feront certainement pas confiance. Le refactoring est censé porter sur de petits changements sécuritaires qui ne modifient pas le comportement. Il y a des développeurs pour qui l'idée même d'un "changement qui ne modifie pas le comportement" est inconcevable . Je ne sais pas quoi suggérer dans ces cas. Je pense que vous devez essayer de travailler dans des domaines qui ne les intéressent pas. J'ai été surpris d'apprendre que ce type peut durer longtemps,

Pourquoi ne faites-vous pas les deux, mais avec des commits séparés?

Vos pairs ont un point. Une révision de code devrait évaluer le code qui a été modifié par quelqu'un d'autre. Vous ne devez pas toucher le code que vous révisez pour quelqu'un d' autre que votre rôle qui sollicite en tant que réviseur.

Mais si vous voyez un certain nombre de problèmes criants, vous pouvez suivre deux options.

1. Si l'examen du code s'est bien déroulé, autorisez la validation de la partie que vous avez révisée, puis refactorisez le code sous un second enregistrement.

2. Si l'examen du code posait des problèmes nécessitant une correction, demandez au développeur de procéder à une refactorisation en fonction des recommandations de Resharper.

Personnellement, je déteste l’idée d’examiner les codes postaux. La révision du code doit avoir lieu au fur et à mesure que vous modifiez le code. Ce dont je parle bien sûr ici, c’est de la programmation en binôme. Lorsque vous vous associez, vous obtenez l'examen gratuit et vous obtenez un meilleur examen du code. Maintenant, j'exprime mon opinion ici, je sais que d'autres partagent cela, il y a probablement des études qui le prouvent.

Si vous pouvez amener vos réviseurs de code à s'associer, l'élément combatif de la révision de code devrait s'évaporer. Lorsque vous commencez à effectuer un changement incompris, la question peut être posée au point de changement, discutée et les alternatives explorées, ce qui peut conduire à une meilleure refactorisation. Vous obtiendrez une révision de code de qualité supérieure, car la paire sera en mesure de comprendre la portée plus large du changement et de ne pas trop se concentrer sur les changements ligne par ligne, ce que vous obtenez avec la comparaison de code côte à côte.

Bien sûr, cela ne constitue pas une réponse directe au problème des refactorisations sortant du cadre du changement sur lequel on travaille, mais je m'attendrais à ce que vos pairs comprennent mieux le raisonnement qui sous-tend les changements s’ils existaient au moment où vous les apportiez.

En passant, en supposant que vous exécutiez un TDD ou une forme de refactor vert-rouge, vous pouvez utiliser la technique de couplage ping-pong pour vous assurer de l’engagement de vos pairs. Simplement expliqué lorsque le pilote est pivoté pour chaque étape du cycle RGR, c'est-à-dire que la paire 1 écrit un test qui a échoué, la paire 2 le répare, la paire 1 est refactée, la paire 2 écrit un test qui échoue ... et ainsi de suite.

Ce problème reflète probablement un problème beaucoup plus important lié à la culture organisationnelle. Les gens semblent plus intéressés par "faire son travail" correctement que par rendre le produit meilleur. Probablement cette société a-t-elle une culture du "blâme" au lieu d'une culture collaborative et les gens semblent plus intéressés par la couverture que par une vision globale du produit / de l'entreprise .

À mon avis, vous avez tout à fait raison, les personnes qui révisent votre code ont complètement tort s’ils se plaignent parce que vous "touchez" des choses extérieures à "votre mission", essayez de convaincre ces personnes sans jamais être contre vos principes. Pour moi, c’est la qualité la plus importante d'un vrai professionnel.

Et si la bonne chose vous donne-t-elle de mauvais chiffres dans une société stupide pour évaluer votre travail, quel est le problème? Qui veut "gagner" ce jeu d'évaluation dans une entreprise insensée? vous êtes fatigué, trouvez un autre endroit, mais jamais, ne soyez jamais contre vos principes, c'est le meilleur que vous avez.

Parfois, la refactorisation est une mauvaise chose. Pas pour les raisons que vos réviseurs de code donnent, cependant; on dirait qu'ils ne se soucient pas vraiment de la qualité du code, et vous vous en souciez, ce qui est génial. Mais il y a deux raisons qui devraient vous empêcher de refactoriser : 1. Vous ne pouvez pas tester les modifications que vous avez apportées avec des tests automatisés (tests unitaires ou autres) ou 2. Vous apportez des modifications à du code que vous ne comprenez pas très bien. bien; à- dire, vous n'avez pas le domaine des connaissances spécifiques à savoir les changements que vous devez faire.

1. Ne refactorisez pas lorsque vous ne pouvez pas tester les modifications apportées avec les tests automatisés.

La personne qui effectue votre révision de code doit être capable de s’assurer que les modifications que vous avez apportées n’écrasent rien de ce qui fonctionnait auparavant. C'est la principale raison de laisser le code fonctionnel seul. Oui, il serait certainement préférable de reformuler cette fonction longue de 1 000 lignes (c'est vraiment une abomination!) En un tas de fonctions, mais si vous ne pouvez pas automatiser vos tests, il peut être très difficile de convaincre les autres que vous avez tout fait. droite. J'ai certainement fait cette erreur avant.

Avant de refactoriser, assurez-vous qu'il existe des tests unitaires. S'il n'y a pas de tests unitaires, écrivez-en! Puis refaites le test et vos réviseurs de code n'auront aucune excuse (légitime) pour être contrarié.

Ne refactorisez pas les éléments de code qui nécessitent des connaissances spécifiques à un domaine que vous n'avez pas.

Le lieu où je travaille a beaucoup de code de génie chimique lourd. La base de code utilise les idiomes communs aux ingénieurs chimistes. N'effectuez jamais de modifications idiomatiques dans un champ. Il peut sembler une bonne idée de renommer une variable appelée xà molarConcentration, mais devinez quoi? Dans tous les textes de chimie, la concentration molaire est représentée par un x. Renommer cela rend plus difficile de savoir ce qui se passe réellement dans le code pour les personnes dans ce domaine.

Au lieu de renommer les variables idiomatiques, mettez simplement des commentaires expliquant ce qu’elles sont. S'ils ne sont pas idiomatiques, veuillez les renommer. Ne laissez pas le i, j, k, x, y, etc. flottant autour quand de meilleurs noms fonctionneront.

Règle de base pour les abréviations: si, lorsque les gens parlent, ils utilisent une abréviation, il est normal d’utiliser cette abréviation dans le code. Exemples de la base de code au travail:

Nous avons les concepts suivants, que nous abrégeons toujours lorsque nous en parlons: "Domaine de préoccupation" devient "AOC", "Explosion de nuages ​​de vapeur" devient VCE, ce genre de choses. Dans notre base de code, une personne a refactorisé toutes les instances appelées aoc dans AreaOfConcern, VCE vers vaporCloudExplosion, nPlanes vers confiningPlanesCount ..., ce qui rendait le code beaucoup plus difficile à lire pour les personnes possédant les connaissances spécifiques du domaine. Je suis coupable de faire des choses comme ça aussi.

Cela pourrait ne pas s'appliquer du tout à votre situation, mais je réfléchis à la question.

Cette question a maintenant deux problèmes distincts: la facilité d’examen des modifications apportées aux révisions de code et le temps consacré à la refactorisation.

Là où je travaille, il y a une pratique de développement qui cause de fortes frictions - la révision de code (CR). Chaque fois que je change quelque chose qui n'entre pas dans le cadre de ma "tâche", mes critiques me reprochent de rendre le changement plus difficile à contrôler.

Comme d'autres réponses l'ont déjà dit, pouvez-vous séparer les archivages de refactorisation des archivages de modification de code (pas nécessairement en tant qu'examens séparés)? Selon l'outil que vous utilisez pour réviser le code, vous devriez être en mesure d'afficher les différences entre des révisions spécifiques uniquement (c'est ce que fait Atlassian's Crucible).

(2) Personne ne vous considérera comme favorable à la publication de "refactoring" de CR que vous n'étiez pas censés faire. Un CR a un certain surcoût et votre responsable ne veut pas que vous perdiez votre temps en refactoring. Lorsque le changement que vous êtes censé apporter est intégré, le problème est minimisé.

Si la refactorisation est simple et rend le code plus facile à comprendre (ce qui est tout ce que vous devriez faire s'il ne s'agit que de refactoriser), alors la révision du code ne devrait pas prendre beaucoup de temps et devrait représenter un surcoût minime qui doit venir et regarder à nouveau le code à l'avenir. Si vos patrons ne sont pas en mesure de le faire, vous devrez peut-être les orienter doucement vers des ressources expliquant pourquoi la règle du scoutisme conduit à une relation plus productive avec votre code. Si vous parvenez à les convaincre que le "temps perdu" vous permettra d'économiser deux, cinq ou dix fois plus tard lorsque vous / quelqu'un d'autre reviendrez travailler plus sur ce code, alors votre problème devrait disparaître.

La question est exacerbée par Resharper, car chaque nouveau fichier que j’ajoute au changement (et je ne peux pas savoir à l’avance quels fichiers seraient modifiés) est généralement semé d’erreurs et de suggestions, dont la plupart sont exactes et méritent totalement. fixation.

Avez-vous essayé de porter ces problèmes à l'attention de vos collègues et de discuter de la raison pour laquelle ils méritent d'être résolus? Et est-ce que l'un d'entre eux peut être corrigé automatiquement (en supposant que vous ayez une couverture de test suffisante pour confirmer que vous n'avez pas cassé des données avec le refactoring automatisé)? Parfois, cela ne vaut pas la peine d’effectuer une refactorisation s’il s’agit vraiment de choses délicates. Est-ce que toute votre équipe utilise ReSharper? Si tel est le cas, avez-vous une politique partagée sur les avertissements / règles appliqués? Si ce n'est pas le cas, vous devriez peut-être à nouveau montrer où l'outil vous aide à identifier les zones du code qui sont des sources possibles de problèmes futurs.

Je me souviens d’il ya 25 ans, environ, du code de «nettoyage» sur lequel je travaillais à d’autres fins, principalement en reformatant les blocs de commentaires et les alignements de tabulations / colonnes pour rendre le code net et donc plus facile à comprendre (aucun changement fonctionnel réel). . Je suis d' aimer le code qui est propre et bien ordonné. Eh bien, les développeurs principaux étaient furieux. En fait, ils ont utilisé une sorte de comparaison de fichiers (diff) pour voir ce qui avait changé, par rapport à leurs copies personnelles du fichier. À présent, cela donnait toutes sortes de faux positifs. Je devrais mentionner que notre bibliothèque de code se trouvait sur un ordinateur central et n’avait pas de contrôle de version - vous avez essentiellement remplacé tout ce qui se trouvait là, et c’était tout.

La morale de l'histoire? Je ne sais pas. Je suppose que c'est que parfois vous ne pouvez plaire à personne, sauf à vous-même. Je ne perdais pas de temps (à mes yeux) - le code nettoyé était plus facile à lire et à comprendre. C'est simplement que les outils de contrôle des modifications primitifs utilisés par d'autres leur donnent un travail ponctuel supplémentaire. Les outils étaient trop primitifs pour ignorer les espaces / tabulations et commentaires redistribués.

Si vous pouvez scinder la modification demandée et le refactor non demandé en (beaucoup) de commits distincts, comme indiqué par @Useless, @Telastyn et autres, c'est ce que vous pouvez faire de mieux. Vous travaillerez toujours sur un seul CR, sans la charge administrative liée à la création d'un "refactoring". Gardez juste vos commits petits et concentrés.

Au lieu de vous donner quelques conseils sur la façon de procéder, je préfère vous indiquer une explication beaucoup plus grande (en fait, un livre) d'un spécialiste. De cette façon, vous pourrez en apprendre beaucoup plus. Lisez Travailler efficacement avec le code hérité (par Michael Feathers) . Ce livre peut vous apprendre à faire le refactoring, entrelacé avec les changements fonctionnels.

Les sujets abordés incluent:

• Comprendre les mécanismes du changement de logiciel: ajout de fonctionnalités, correction de bugs, amélioration de la conception, optimisation des performances
• Obtenir le code hérité dans un faisceau de test
• Écrire des tests qui vous protègent contre l'introduction de nouveaux problèmes
• Techniques pouvant être utilisées avec n'importe quel langage ou plate-forme, avec des exemples en Java, C ++, C et C #
• Identifier avec précision où les modifications de code doivent être effectuées
• Faire face aux systèmes existants qui ne sont pas orientés objet
• Traitement des applications qui ne semblent pas avoir de structure

Ce livre comprend également un catalogue de vingt-quatre techniques de suppression de dépendance qui vous aident à utiliser des éléments de programme de manière isolée et à effectuer des modifications plus sûres.

Moi aussi, je suis un fervent partisan de The Boyscout Rule et du Refactoring opportuniste. Le problème est souvent d'obtenir l'adhésion de la direction. La refactorisation comporte à la fois des risques et des coûts. Ce que la direction oublie souvent, c’est que la dette technique fait de même.

C'est la nature humaine. Nous sommes habitués à traiter de vrais problèmes, sans chercher à les prévenir. Nous sommes réactifs, pas proactifs.

Le logiciel est intangible et il est donc difficile de comprendre que, comme une voiture, il doit être réparé. Aucune personne raisonnable n'achèterait une voiture et ne la réparera jamais. Nous reconnaissons qu'une telle négligence réduirait sa longévité et, à long terme, coûterait plus cher.

Malgré le fait que de nombreux gestionnaires comprennent cela, ils sont tenus responsables des modifications apportées au code de production. Il y a des politiques qui permettent de laisser plus facilement tranquille. Souvent, la personne qui a besoin de convaincre est au-dessus de votre supérieur hiérarchique et elle ne veut tout simplement pas se battre pour que vos "bonnes idées" entrent en production.

Pour être honnête, votre responsable n’est pas toujours convaincue que votre remède est aussi efficace que vous le pensez. (Huile de serpent?) Il y a une vente. C'est à vous de l'aider à voir les bénéfices.

La direction ne partage pas vos priorités. Mettez votre chapeau de gestion et voyez avec les yeux de la direction. Vous trouverez plus probablement le bon angle. Être persistant. Vous effectuerez plus de changements en ne permettant pas à la première réponse négative de vous décourager.

Si vous pouvez diviser la modification demandée et le refactor non demandé en deux demandes de modification distinctes, comme indiqué par @ GlenH7, c'est ce que vous pouvez faire de mieux. Vous n'êtes alors pas "le gars qui perd son temps" mais "le gars qui travaille deux fois plus dur".

Si vous vous retrouvez souvent dans une situation où vous ne pouvez pas les séparer, car le travail demandé nécessite maintenant le refactor non demandé à compiler, je vous suggérerais d'insister pour que vous disposiez d'outils permettant de vérifier automatiquement les normes de codage, en utilisant les arguments exposés ici . les avertissements seuls peuvent constituer un bon candidat, je ne le connais pas bien). Ces arguments sont tous valables, mais vous pouvez en tirer un avantage supplémentaire: si vous avez maintenant ces tests, il est de votre devoir de réussir les tests pour chaque commit.

Si votre entreprise ne veut pas "perdre de temps en refactorisation" (un signe médiocre de sa part), elle doit alors vous fournir un fichier "Suppression du message d'avertissement" (chaque outil possède son propre mécanisme) afin que vous ne soyez plus ennuyé. de tels avertissements. Je dis cela pour vous fournir des options pour différents scénarios, même dans le pire des cas. Il est également préférable d’avoir des accords clairement définis entre vous et votre entreprise, également sur la qualité de code attendue, plutôt que des hypothèses implicites de chaque côté.