Peut-on modifier le style de code d'un projet open source qui ne respecte pas les meilleures pratiques?

38

Récemment, je suis tombé sur un certain nombre de projets Ruby Open Source (ou la majorité de projets Ruby) sur GitHub qui, lorsqu'ils sont vérifiés à l'aide d'un outil d'analyse de code comme Rubocop , génèrent de nombreuses infractions .

Maintenant, la plupart de ces infractions comprennent l' utilisation des guillemets doubles au lieu de guillemets simples (lorsqu'ils ne sont pas d'interpolation), ne suit pas les 2 places par règle de niveau, dépassant la règle de longueur de ligne de 80 caractères, ou à l' aide {et }des blocs multi-lignes.

[Le] style Guide de Ruby recommande les meilleures pratiques afin que les programmeurs Ruby du monde réel puissent écrire du code pouvant être géré par d'autres programmeurs Ruby du monde réel. ~ Source: Ruby Style Guide

Bien qu’ils soient petits et faciles à corriger, convient-il de changer le style de codage d’un projet open source en corrigeant les infractions et en effectuant une demande d’extraction? Je reconnais que certains projets, tels que Rails, n'acceptent pas les modifications esthétiques et que certains sont trop volumineux pour être "réparés" d'un seul coup (Rails, par exemple, génère plus de 80 000 infractions lorsque Rubocop est exécuté - indépendamment de leur propre petit jeu de codage conventions à suivre pour contribuer). Après tout, le Ruby Style Guide existe avec des outils comme Rubocop.

Les gens apprécient la cohérence, alors ce genre de changement est une bonne chose pour la communauté Ruby en général, non?

[Les auteurs du Ruby Style Guide] n’ont pas mis au point toutes les règles qui viennent de nulle part. Celles-ci sont principalement basées sur ma longue carrière d’ingénieur logiciel, sur les commentaires et suggestions de membres de la communauté Ruby et sur diverses autres solutions. ressources de programmation Ruby très réputées, telles que "Programmation Ruby 1.9" et "Le langage de programmation Ruby". ~ Source: Ruby Style Guide

Le fait de ne pas suivre les conventions de style de codage communautaire et les meilleures pratiques n’encourage-t-il pas les mauvaises pratiques?

raf
la source
3
Question similaire: Devrais-je refactoriser une classe F du code climatique? , mais avec plus d’attention sur l’architecture.
amon
5
Bon nombre de ces problèmes de style semblent plutôt mineurs.
JL235
4
@MathewFoscarini non, ce qu'ils demandent, c'est "si j'achète un pistolet radar, puis-je décider quelles sont les lois locales en matière de conduite?"
Jon Hanna

Réponses:

65

Demandez aux mainteneurs.

Le style de codage est une discussion assez subjective, et des règles comme une longueur de ligne maximale de 80 caractères sont assez subjectives - bien qu'un accord général soit trouvé pour dire que les lignes plus courtes sont plus lisibles, 80 peut être trop restrictif pour certains avec les tailles d'écran et les IDE actuels.

D'autres règles peuvent aussi être ignorées volontairement. Par exemple, un développeur peut envisager l'utilisation globale des doubles guillemets mieux pour lui et être prêt à accepter le "risque" d'interpolation accidentelle et une augmentation extrêmement faible du temps d'analyse.

De plus, de nombreux responsables n'aiment pas les changements de style de code importants, car ils sont fastidieux à réviser et risquent de générer des erreurs. Par exemple, une chaîne pourrait être basculée sur guillemet simple, même si elle contenait une interpolation délibérée et aurait dû utiliser des guillemets doubles. Les responsables préfèrent effectuer des nettoyages de style tout en travaillant sur ce code afin de pouvoir vérifier que les modifications de style n'introduisent pas de nouveaux bogues.

johannes
la source
34
De plus, de nombreux responsables n'aiment pas les changements importants qui ne sont pas strictement nécessaires car ils ont tendance à créer des conflits de fusion qui ne sont pas non plus strictement nécessaires.
Jan Hudec
4
Vous perdrez la fonction annotate (qui crée la ligne de code) dans le contrôle de source. S'il y a un bogue, il est difficile de blâmer où il a été introduit et de suivre s'il y a plus d'erreur de ce type.
peer
4
En outre, si les conventions spécifiques au projet sont cohérentes, vous pouvez créer une configuration spécifique au projet pour l'outil de vérification de la convention et proposer les conventions config en tant que demande d'extraction. Ainsi, les responsables peuvent vérifier leur projet en utilisant leurs configurations. (Je ne sais pas si cela est possible avec l'outil que vous avez utilisé.)
Peter Kofler
+1 sur les conflits de fusion. Je viens d'accepter un remaniement du style de codage et j'aurais souhaité ne pas l'avoir fait, car cela a provoqué des conflits de fusion avec les relations publiques d'autres personnes. C'est facile à résoudre mais j'aurais préféré le faire pièce par pièce
Daenyth le
C’est l’EDI qui est restrictif s’il ne permet pas d’afficher deux fichiers séparés côte à côte, pas la faute de code abrégé.
unperson325680
48

Vous semblez être motivé en grande partie par le respect de l'autorité de l'outil Rubocop et du Ruby Style Guide, que les responsables ne peuvent pas partager. Ils ont déjà leur propre style et y sont habitués. Tout changement affecterait donc toutes les personnes travaillant sur le projet, ce qui représente beaucoup de travail, surtout si le projet est volumineux.

Considérez les motivations des mainteneurs. Ils veulent (probablement) que de nouvelles personnes les rejoignent en soumettant des corrections de bugs, des rapports de bogues, du code de travail et une documentation bien écrite. Si vous vous présentez et dites "Vous avez des infractions en rubocop", ils ne vont pas penser "Oh, bon, quelqu'un de nouveau pour aider à partager le fardeau", ils vont penser: "Qui est ce type? Pourquoi nous dit-il Que faire?".

Les projets open source ont tendance à être des méritocraties: vous obtenez un respect basé sur la qualité de votre travail. Si vous vous présentez et faites de grandes choses, puis que vous soulevez vos problèmes de style, ils seront plus enclins à écouter, même s'ils pourraient toujours refuser. Il y a une source ouverte disant "Parlez, c'est pas cher, montrez-moi le code".

Michael Shaw
la source
1
Meilleure réponse. Vous devez respecter les efforts de ceux qui sont venus avant lors de la soumission de demandes d'attraction. Si vous modifiez le style de projet sous les pieds de tous les développeurs existants, en particulier de ceux qui ont un "rang" plus élevé que vous dans la méritocratie, il leur est difficile de se souvenir des nouvelles règles que vous introduisez. Cela nuirait à leur capacité à maintenir le projet tel qu'il a été. De plus, si vous étiez déjà actif dans le développement du projet, vous connaîtrez probablement les réflexions du mainteneur sur les changements stylistiques et le meilleur moment du cycle de vie du projet pour les introduire.
Chris Keele
1
Exactement. Par exemple, le projet Linux, en dépit d’un guide de style assez strict pour les nouveaux éléments de code, ne vous permettra pas de simplement soumettre une grosse demande d’aiguillage en résolvant des problèmes de style, car il bloque la fusion. Il vous demande même de conserver le style local, même si cela signifie que cela va à l'encontre du guide de style du projet.
Miles Rout
25

Le pragmatisme sur le dogme, toujours. Les guides de style de codage sont une forme de mal particulièrement insidieuse qui détourne l'attention des préoccupations architecturales vers des absurdités frivoles comme des guillemets simples / doubles. Posez-vous la question suivante: cela fait-il vraiment une différence?

Ils peuvent être bons jusqu'à un certain point, mais à la seconde où vous les traitez avec une ferveur presque religieuse, vous allez trop loin. Ce sont des lignes directrices, des suggestions, des opinions, PAS des faits.

Devraient-ils simplement être ignorés alors? Non, il est judicieux d'utiliser les outils pour avoir une idée générale de ce qui doit être examiné, mais rien de plus.

C'est étonnant de constater à quelle fréquence les types juniors confondent leurs opinions sur des faits.

Baweaver
la source
11
Il convient d'ajouter que les changements de reformatage ont tendance à créer des conflits de fusion, ce qui constitue une très bonne raison pour la plupart des responsables de la maintenance de détester le fait de haïr le reformatage simplement pour le plaisir de le reformer.
Jan Hudec le
13

Vous pouvez extraire ces modifications uniquement s'il existe un problème non résolu pour résoudre le formatage. Sinon, démarrez votre propre branche, et si l'auteur constate que davantage de personnes utilisent votre branche simplement parce qu'elle est plus lisible. Ils fusionneront dans la branche eux-mêmes, mais soyez prêt à maintenir votre branche en fusionnant les mises à jour et en corrigeant constamment le formatage.

Si le projet n'est pas assez important pour que vous puissiez continuer à entretenir votre propre succursale, il ne valait pas la peine de le nettoyer en premier lieu.

Les demandes d'extraction peuvent être traitées personnellement par l'auteur. Ce n'est pas un mécanisme de critique, et reformater tout le code pourrait être considéré comme une critique.

Si vous ne souhaitez pas conserver votre propre branche, mais que vous souhaitez contribuer à un projet. Ouvrez un nouveau problème et décrivez pourquoi le format actuel vous cause des problèmes, puis proposez à l'auteur de résoudre le problème. Si l'auteur accepte, il vous attribue le problème et vous avez maintenant la permission de faire une demande de tirage.

Vous avez abordé un sujet qui, je le reconnais également, est un problème répandu sur GitHub . Mis à part le formatage, il existe un certain nombre de projets qui utilisent de manière incorrecte les annotations et qui causent des dégâts avec de nombreux IDE. Je peux penser à trois projets largement populaires qui utilisent de manière incorrecte des indicateurs obsolètes qui propagent des messages d’avertissement dans mon IDE. J'ai envoyé des demandes d'extraction pour les corriger, mais les auteurs n'utilisent pas le même IDE. Par conséquent, les demandes d'extraction sont ignorées.

La ramification, la fusion et la correction semblent être la seule solution.

Réactionnel
la source
À votre avis, considéreriez-vous les avantages pour les futurs contributeurs comme une "excuse" valable pour une infraction de type Pull Request? Par exemple, pour que les futurs contributeurs n'aient pas à s'inquiéter de parcourir l'intégralité de la base de code afin de saisir son style de codage.
raf
@RafalChmiel Lorsqu'un auteur accepte une demande d'extraction, la personne qui l'a envoyé est ajoutée au référentiel et publiquement listée en tant que contributeur au projet et elle reste répertoriée en tant que contributeur. Lors de l'examen d'une demande de tirage, l'auteur gardera cela à l'esprit lorsqu'il décidera de l'accepter. Gardez cela à l'esprit lorsque vous envoyez des demandes d'extraction. Faire des choses dignes d'être un contributeur.
Reactgular
Ce que je voulais dire, c'est que le fait de profiter des futurs contributeurs en corrigeant les infractions serait un motif valable pour fusionner un RP avec de tels correctifs. Pourquoi le mainteneur devrait-il fusionner un tel PR? Peut-être que le libellé de ma question était un peu décalé.
raf
2
@RafalChmiel tout ce que vous déclarez comme raison n'est que votre opinion. S'il s'agit d'un code offensant qui ressemble à un taureau, alors écrivez vos peurs dans un numéro. Si l'auteur se défend contre un tel tirage, essuyez vos larmes avec un mouchoir en papier.
Reactgular
10

Tiré du site Rubocop lui-même (emphase moi):

Une chose me préoccupe toujours en tant que développeur Ruby: les développeurs Python ont une excellente référence en matière de style de programmation (PEP-8) et nous n’avons jamais de guide officiel documentant le style de codage Ruby et les meilleures pratiques.

Essaye de comprendre:

Il n'y a pas de guide de style Ruby officiel

Je ne dis pas que les guides de style sont mauvais. Non seulement il n'y a pas de guide officiel, mais avoir un guide de style est un choix qui est fait au niveau personnel, du projet, de l'équipe, de l'entreprise. Les guides de style sont, pour utiliser un terme de psychologie, une "norme sociale dans le groupe".

Qu'est-ce que cela signifie pour vous? Eh bien, si vous n'êtes pas considéré comme faisant partie du groupe, cela signifie que tout ce que vous, ou tout autre site Web, déclarez, a peu de chance de tenir le groupe en échec. Par conséquent, si vous n'êtes pas un contributeur actif et respecté à ce projet spécifique, vos suggestions seront probablement ignorées ou, au mieux, rappelleront les considérations précédentes relatives à la rédaction d'un guide de style. Dans le pire des cas, il sera pris comme une insulte ou comme un intrus ou un cycliste qui passe son nez là où il ne doit pas appartenir.

Ne pouvez-vous pas simplement suggérer un guide de style?

En fait, cela semble être ce que vous voulez faire: vous croyez en la valeur des guides de style, vous accordez une grande importance à la cohérence et vous voulez évangéliser pour le dévouement à des directives de style unifiées.

C'est bien, tant que vous êtes vraiment clair, c'est ce que vous êtes et ce que vous voulez accomplir. Si vous croyez dans un guide de style en particulier et croyez qu'il s'agit du One True Style Guide, ou du moins meilleur que tout ce que ces païens sans loi pratiquent, alors c'est très bien aussi.

Mais ce que les gens n’apprécient pas, c’est que leur comportement ne soit pas conforme à des règles non officielles, non contraignantes et en grande partie arbitraires qui proviennent d’une source qu’ils ne considèrent pas comme une autorité légitime. Lorsque c'est ce que vous avez décidé de faire, ou si c'est simplement ce que vous êtes censé faire, vous obtiendrez moins de "traitement au tapis rouge" et davantage de "indigènes en colère avec des lances et une grande marmite bouillante" traitement.

BrianH
la source
3

Pour proposer ici un autre avis, dans de nombreux cas, un tel changement serait le bienvenu. Les projets Open Source ont tendance à avoir de nombreux auteurs. Il n'y a souvent pas de "style de codage"; le style correspond à tout ce que la personne qui a écrit le code en question a utilisé. Si un fichier a été écrit par une personne différente de l'autre, le style peut également être différent. Même dans les projets où il existe un style de consensus, à moins qu'ils ne vérifient ce style régulièrement, il n'est souvent pas utilisé.

D'après mon expérience, un exemple courant est celui où quelqu'un introduit du code par une requête d'extraction de qualité relativement médiocre. Cependant, le code peut fonctionner. Différentes personnes ont des points de vue différents à ce sujet. Certaines personnes refuseront de fusionner une demande de tirage à moins que le style ne soit bon. Certaines personnes ne s'en soucient pas, tant que le code fonctionne. Certaines personnes préfèrent avoir un bon style, mais elles ne veulent pas effrayer les contributeurs avec un tas de commentaires "corrigez les blancs ici" (personnellement, je me sens toujours un peu coupable de faire ces commentaires, même si je sais qu'ils sont pour le meilleur bien de la base de code, car il a l’impression que cela pourrait effrayer le contributeur).

Donc, ne supposez pas directement que le style que vous voyez est celui que le projet veut. En fait, cela peut probablement être généralisé à la contribution à l'open source en général: ne supposez pas que le code d'un projet open source est le code qu'il veut .

Vous devez cependant être conscient de certaines choses:

  • Certaines personnes sont religieuses à propos du style. S'il devient clair qu'ils ne veulent pas bouger, alors ne vous embêtez pas.

  • C'est un énorme problème de bikeshed . Tout le monde et son frère ont un avis sur ces choses. Il peut être difficile d'obtenir une telle demande d'extraction fusionnée pour cette raison.

  • Il peut également être difficile de fusionner une telle demande d'extraction, car elle entraînera très rapidement des conflits de fusion. En gros, chaque fois que vous modifiez une partie de la base de code que vous avez modifiée, même de manière triviale.

Je resterais avec l'approche "demander d'abord". S'ils sont ouverts à cela et que vous êtes prêt à vous en tenir à la demande de tirage, alors allez-y.

Asmeurer
la source
2

J'avais l'habitude d'avoir un bon guide de style, mais étant donné l'état des choses dans Ruby, "je suis passé à autre chose".

En gros, je vis avec ce avec quoi je travaille et suis les conventions générales que j’ai apprises de plusieurs métiers.

Pour Ruby, qui est ma langue de choix, j'ai aussi (dans ma tête) divisé mon style en universellement accepté, généralement accepté, mes préférences et mes meilleures pratiques. Choses universellement acceptées Je pourrais soumettre une modification dans le cadre d'une demande de modification pour une demande de problème ou de branche de fonctionnalité.

Exemples de chaque style (à mon avis):

Universellement accepté pour Ruby:

  • espaces non tabulations
  • deux tirets spatiaux
  • method_names_use_underscores
  • Les constantes commencent par Caps.

Généralement accepté:

  • N'utilisez pas de parenthèses si ce n'est pas nécessaire
  • Utiliser { }pour les blocs d'une ligne et do endpour les blocs de plusieurs lignes.
  • CONSTANTS_ARE_IN_ALL_CAPS
  • Utilisez des déclarations prédicatives ( cond ? true : false) avant if thensi l'expression tient sur 1 ligne.

Préférences personnelles:

  • Longueur de ligne de 120 lignes de caractères
  • les whendéclarations de cas sont en retrait 2 de la déclaration de cas.
  • Utilisez des guillemets simples sauf si vous avez besoin de doubles

Pas d'accord:

  • Déclarations de cas
  • Longueur de la ligne

Les meilleures pratiques:

  • petites méthodes, <= 5 lignes si possible.
  • petites classes, <= 100 lignes si possible.
  • Eviter les constantes
  • Code a des tests

Enfin, comme d'autres l'ont détaillé, vous devriez demander en premier. En fin de compte, la clé du style est la communication entre les développeurs et la sensibilité aux autres. Par exemple, si je souhaite modifier le style d'un projet open source, je vais souvent faire une demande d'extraction pour une ou deux fonctionnalités réelles ou des corrections de bugs. Une fois que le mainteneur me connaît et voit que j'ai contribué, alors je pourrais suggérer des changements de style. Je ne leur suggère que si. Par exemple, "lors de la création d’une autre fonctionnalité sur le projet x, j’ai remarqué que vous aviez 4 espaces de retrait dans quelques fichiers et je me demandais si je pouvais les changer en 2?"

Michael Durrant
la source
1

Bien qu’ils soient petits et faciles à corriger, pensez-vous qu’il est acceptable de changer le style de codage d’un projet open source en corrigeant les infractions et en effectuant une requête Pull?

Bref non!

Bien sûr, je suppose ici que ce ne sont que des problèmes de style et qu'il ne s'agit pas de bugs réels. Il serait toujours judicieux de demander à ce dernier de tirer. (IMO.) Je suppose également que vous êtes étranger au projet et pas en contact avec les personnes qui le maintiennent déjà.

Cependant, parmi toutes les langues, il y a toujours des personnes dont les préférences diffèrent du guide de style, pour le meilleur ou pour le pire, et qui essaient d'imposer ces changements de style à des personnes que vous ne connaissez pas , et à un projet dont vous ne faites pas partie. de si rien d'autre ne semble un peu grossier. Après tout, que réalisez-vous (en réalité) si la demande était acceptée? Selon toute vraisemblance, si les membres d’un projet souhaitaient changer le style de manière différente, ils l’auraient déjà fait - et tout ce que vous feriez avec cette demande serait de leur imposer un style qui ne fonctionnerait pas nécessairement mieux. pour les membres existants.

Cela change légèrement si vous êtes prêt à contribuer au projet autrement que par des corrections de style. Je dirais que si vous souhaitez ouvrir un dialogue avec les responsables de la maintenance sur la manière dont vous souhaitez travailler sur le projet mais que vous trouvez difficile de trouver un style non standard, c'est très bien. Mais je n’aurais vraiment pas voulu créer aveuglément des demandes d’attraction pour un ensemble de projets ne contenant que des changements de style!

berry120
la source
1

N'IMPORTE QUELLE modification est susceptible d'introduire des bogues, même en modifiant la mise en forme typographique du code.
Par conséquent, aucune modification ne doit être apportée au code sauf s'il existe une analyse de rentabilisation valide. "Mais le code n'a pas l'air sympa" ou "le code ne respecte pas les normes de codage" n'est pas une analyse de rentabilisation valable. Le risque est simplement trop grand.
De toute façon, si vous apportez des modifications majeures à un fichier source, il peut être acceptable d’aligner l'ensemble du fichier sur les normes, mais vous ferez probablement de petites modifications, auquel cas il est presque universellement préférable de conserver vos propres modifications. aligner avec le code existant même si ce code existant ne respecte pas les normes de codage.
Heck, le code pourrait bien être le résultat d'un générateur de code et parfois être régénéré. Les générateurs de code sont connus pour produire du code laid ...

jwenting
la source
1

J'ai quelques projets .NET qui ont connu un succès modéré, et j'ai eu quelques relations publiques avec des personnes qui semblent avoir étudié le code avec ReSharper et StyleCop et "réparé" un tas de choses. Je n'accepte pas ces relations publiques pour deux raisons:

  • Bien que de nombreux changements soient bénéfiques, certains d'entre eux ont un impact négatif sur le code, généralement les performances, et sélectionner les meilleurs éléments est un travail trop ardu.
  • Même si tous les changements étaient bénins ou même bénéfiques, chaque changement dans chaque fichier de chaque commit doit être revu, et encore une fois, cela prend trop de temps.

Cela dit, si quelqu'un souhaitait ajouter des vérifications d'erreur ou des commentaires de meilleure qualité, j'accepterais cette relation sans attendre.

Mark Rendle
la source
-1

Vous devez déterminer si les responsables considèrent ces violations de style de codage comme un défaut ou si le projet a une norme différente pour le style de codage.

Si la norme est différente, vous pouvez proposer de l’aider à la documenter ou à la formaliser. Ensuite, il se peut que certaines violations de ce style se produisent et que vous puissiez les corriger.

Tim
la source
cela ne semble rien offrir de précieux par rapport à une autre réponse postée plusieurs heures avant celle-ci
Gnat,