Dois-je insister pour que nous effectuions des révisions de code avant de réintégrer le tronc?

Nouvelle publication demandée à StackOverflow:

Je travaille dans un petit temps de développement avec un temps de développement très limité. Nous développons un outil important pour le résultat de notre travail, mais non utilisé quotidiennement. Je suis la seule personne de l'équipe à avoir une formation de programmeur.

Mon problème est que j'ai poussé pour les revues de code avant de fusionner de nouveau dans le tronc pendant plus d'un an. Tout le monde était d'accord, mais ce n'est que mon code qui a été révisé. De retour de longues vacances, je reviens à un coffre avec des commentaires de code car "c'est une solution laide - à retirer le plus tôt possible" et "solution rapide". Ce qui est également nouveau, c'est qu'un gars a été nommé responsable de l'outil. (Un rôle qui m'a été proposé en premier mais que j'ai refusé pour une raison non liée au travail.) Et il pense que c'est une bonne façon de travailler: comme nous avons un temps si limité à développer, nous devrions couper les coins comme ça.

Ma préoccupation est que les autres développeurs écrivent du code laid: cassant souvent l'encapsulation, écrivant d'énormes classes, ajoutant des classes internes à des endroits étranges, ayant peu ou pas de tests unitaires, etc. Il sera finalement impossible de développer davantage l'outil.

Dois-je insister pour que nous effectuions des révisions de code avant de fusionner de nouveau dans le tronc ou suis-je juste une chienne de qualité de code?

j'ai été dans des situations similaires auparavant et à mon humble avis, cela dépend de "dois-je maintenir le code".

Si je dois maintenir le code, je veux un code de haute qualité, personnellement, je n'ai pas besoin de révisions de code pour chaque commit (c'est-à-dire que les programmeurs peuvent décider eux-mêmes si certains codes ont besoin d'une révision ou non) mais si la lisibilité / maintenabilité souffre cela pourrait être en ordre.

En lisant ceci:

Ma préoccupation est que les autres développeurs écrivent du code laid: cassant souvent l'encapsulation, écrivant d'énormes classes, ajoutant des classes internes à des endroits étranges, ayant peu ou pas de tests unitaires, etc. Il sera finalement impossible de développer davantage l'outil.

Je pense que votre problème est plus important que de simples revues de code. Vous semblez manquer quelques lignes directrices et / ou elles ne sont pas mises en œuvre. Ne pas avoir / peu de tests unitaires peut être une mauvaise idée mais dépend du cas spécifique. Cependant, breaking encapsulation, writing huge classes, ...faites vraiment du code sujet aux bogues, ce qui devrait définitivement être corrigé.

Je pense que les révisions de code et le maintien de certaines directives de codage sont une bonne idée, mais je pense que le faire pour chaque enregistrement est une perte de temps. C'est une bonne idée lors de la création d'une équipe et avec de jeunes programmeurs, mais les programmeurs expérimentés peuvent penser par eux-mêmes et, éventuellement, vous devrez leur faire confiance. Cela dit - vous pouvez faire des revues de code périodiques pour rafraîchir les choses, mais regarder chaque ligne de code qui entre dans votre VCS en fait vraiment trop.

Et un petit commentaire concernant les correctifs de votre collègue - parfois, faire un vilain correctif est la bonne solution . Il se peut que ce code particulier ne soit pas suffisamment important pour investir beaucoup de temps, il se peut que la solution simple soit suffisamment bonne et qu'il soit préférable d'investir du temps dans d'autres choses. Rendre votre code "joli" n'est pas votre objectif principal en tant que programmeur. Votre objectif principal est de livrer et d'obséder sur chaque ligne de code ne vous y mènera tout simplement pas.

Ce que j'essaie de dire, c'est que vous devez choisir vos combats. C'est OK de perdre la bataille contre une classe d'utilité insignifiante pour gagner la guerre de livraison (ou cette guerre de sous-système VRAIMENT IMPORTANTE, d'ailleurs).

Si le programme en question n'est pas un prototype jetable unique, je pense que les révisions de code devraient être obligatoires pour chaque enregistrement.

Les développeurs seniors pourraient avoir le privilège des enregistrements non révisés, une fois qu'ils sont suffisamment consciencieux pour demander un examen, le cas échéant.

Je ne suis pas sûr que la révision du code soit la réponse jusqu'à ce que quelqu'un commence à appliquer de meilleures normes de codage. Quelqu'un écrit du code merdique, le commente / l'admet et le vérifie de toute façon. À quoi sert une révision si quelqu'un veut rejeter le code, mais cela vous mettra en retard?

Vous allez devoir: établir des normes, superviser de plus près les principaux coupables et leur taper dans la tête qu'un bon code ne prend pas toujours plus de temps à écrire. Ils doivent cesser d'utiliser les délais comme excuse pour refuser de changer de mauvaises habitudes.