¿Está bien hacer cambios de estilo de codificación en un proyecto de código abierto que no sigue las mejores prácticas?

38

Recientemente, encontré una serie de proyectos de código abierto Ruby (o la mayoría de ellos era Ruby) en GitHub que, cuando se verifica con una herramienta de análisis de código como Rubocop , crean muchas ofensas .

Ahora, la mayoría de estas ofensas incluyen el uso de comillas dobles en lugar de comillas simples (cuando no es interpolación), no seguir la regla de 2 espacios por nivel, exceder la regla de longitud de línea de 80 caracteres, o usar {y }para bloques de varias líneas.

[La] guía de estilo de Ruby recomienda las mejores prácticas para que los programadores de Ruby del mundo real puedan escribir código que otros programadores de Ruby del mundo real puedan mantener. ~ Fuente: Guía de estilo Ruby

Aunque son pequeños y fáciles de solucionar, ¿es apropiado cambiar el estilo de codificación de un proyecto de código abierto arreglando los delitos y haciendo una solicitud de extracción? Reconozco que algunos proyectos, como Rails, no aceptan cambios cosméticos y algunos son demasiado grandes para "arreglarlos" de una vez (Rails, por ejemplo, genera más de 80,000 delitos cuando se ejecuta Rubocop, independientemente de que tienen su propio conjunto pequeño de codificación convenciones a seguir al contribuir). Después de todo, la Guía de estilo Ruby está allí por una razón junto con herramientas como Rubocop.

La gente aprecia la coherencia, por lo que hacer este tipo de cambios es algo bueno para la comunidad de Ruby en general, ¿verdad?

[El (los) autor (es) de la Guía de estilo de Ruby) no elaboraron todas las reglas de la nada: se basan principalmente en mi extensa carrera como ingeniero de software profesional, comentarios y sugerencias de miembros de la comunidad de Ruby y varios recursos de programación de Ruby de gran prestigio, como "Programming Ruby 1.9" y "The Ruby Programming Language". ~ Fuente: Guía de estilo Ruby

¿Acaso no seguir las convenciones de estilo de codificación de la comunidad y las mejores prácticas básicamente fomenta las malas prácticas?

raf
fuente
3
Pregunta similar: ¿Debería refactorizar una clase F del código climático? , pero con más enfoque en la arquitectura.
amon
55
Muchos de estos problemas de estilo suenan bastante menores.
JL235
44
@MathewFoscarini no, lo que preguntan es: "si compro una pistola de radar, ¿puedo decidir cuáles son las leyes locales de manejo?"
Jon Hanna

Respuestas:

65

Pregunta a los mantenedores.

El estilo de codificación es una discusión bastante subjetiva, y las reglas como la longitud máxima de línea de 80 caracteres son bastante subjetivas, mientras que el acuerdo general debería ser que las líneas más cortas son mejores para leer, 80 podría ser demasiado restrictivo para algunos con los tamaños de pantalla e IDE actuales.

También se pueden ignorar otras reglas a propósito. Por ejemplo, un desarrollador podría considerar mejor el uso global de comillas dobles y estar dispuesto a aceptar el "riesgo" de interpolación accidental y un aumento extremadamente pequeño en el tiempo de análisis.

A muchos mantenedores tampoco les gustan los grandes cambios de estilo de codificación, ya que son aburridos de revisar y existe la posibilidad de que pueda introducir errores. Por ejemplo, una cadena podría cambiarse a comillas simples, aunque contuviera una interpolación deliberada y debería haber estado usando comillas dobles. Los encargados del mantenimiento prefieren realizar limpiezas de estilo mientras trabajan en ese código real para poder verificar que los cambios de estilo no introduzcan nuevos errores.

johannes
fuente
34
A muchos mantenedores tampoco les gustan los grandes cambios que no son estrictamente necesarios porque tienden a crear conflictos de fusión que tampoco son estrictamente necesarios.
Jan Hudec
44
Perderá la función de anotación (que crea la línea de código) en el control de origen, si hay un error es difícil culpar dónde se introdujo y rastrear si hay más de ese tipo de error.
igual
44
Además, si las convenciones específicas del proyecto son consistentes, puede crear una configuración específica del proyecto para la herramienta de verificación de convenciones y ofrecer la configuración de convenciones como solicitud de extracción. Para que los encargados del mantenimiento puedan verificar su proyecto utilizando sus configuraciones. (No sé si esto es posible con la herramienta que utilizó.)
Peter Kofler
+1 en los conflictos de fusión. Acabo de aceptar un retrabajo de estilo de codificación y desearía no haberlo hecho, ya que ha causado conflictos de fusión con las relaciones públicas de otras personas. Es fácil de resolver, pero hubiera
preferido
Es el IDE el que es restrictivo si no permite mostrar dos archivos separados uno al lado del otro, no es culpa del código corto.
unperson325680
48

Parece estar motivado en gran medida por el respeto a la autoridad de la herramienta rubocop y la Guía de estilo Ruby, que los encargados del mantenimiento no pueden compartir. Ya tienen su propio estilo y están acostumbrados, por lo que cualquier cambio afectaría a todos los que trabajan en el proyecto, y eso es mucho trabajo, especialmente si el proyecto es grande.

Considere las motivaciones de los mantenedores. Ellos (probablemente) quieren que nuevas personas se unan a ellos mediante la presentación de correcciones de errores, informes de errores, código de trabajo y documentación bien escrita. Si apareces y dices "Tienes ofensas en rubocop", no van a pensar "Oh, bueno, alguien nuevo para ayudar a compartir la carga", van a pensar "¿Quién es este tipo? ¿Por qué nos está diciendo? ¿qué hacer?".

Los proyectos de código abierto tienden a ser meritocracias: obtienes respeto en función de la calidad de tu trabajo. Si apareces y haces grandes cosas y luego mencionas tus preocupaciones de estilo, es más probable que escuchen, aunque aún podrían decir que no. Hay un código abierto que dice "Talk es barato, muéstrame el código".

Michael Shaw
fuente
1
La mejor respuesta. Debe respetar los esfuerzos de aquellos que vinieron antes al enviar solicitudes de extracción. Cambiar el estilo del proyecto debajo de los pies de todos los desarrolladores existentes, especialmente de todos los que tienen un 'rango' más alto que tú en la meritocracia, les dificulta recordar las nuevas reglas que introduces. Esto dañaría su capacidad para mantener el proyecto de la forma en que lo han sido. Además, si ya estaba activo en el desarrollo del proyecto, probablemente conocería las ideas del encargado del mantenimiento sobre los cambios estilísticos y el mejor punto en el ciclo de vida del proyecto para presentarlos.
Chris Keele
1
Exactamente. Por ejemplo, el proyecto de Linux, a pesar de tener una guía de estilo bastante estricta para las nuevas piezas de código, no le permitirá simplemente enviar una gran solicitud de extracción para solucionar problemas de estilo, ya que molesta con la fusión. Incluso le pide que mantenga el estilo local, incluso si eso significa que va en contra de la guía de estilo del proyecto.
Miles Rout
25

El pragmatismo sobre el dogma, siempre. Las guías de estilo de codificación son una forma de mal especialmente insidiosa que desvía la atención de las preocupaciones arquitectónicas hacia tonterías frívolas como citas simples / dobles. Pregúntese: ¿Realmente hace la diferencia?

Pueden ser buenos hasta cierto punto, pero en el segundo en que los tratas con un fervor casi religioso, has ido demasiado lejos. Son pautas, sugerencias, opiniones, NO hechos.

¿Deberían ser ignorados entonces? No, vale la pena usar las herramientas para tener una idea general de lo que hay que mirar, pero nada más.

Es una maravilla con qué frecuencia los tipos junior confunden opinión con hechos.

baweaver
fuente
11
Vale la pena agregar que los cambios de reformateo tienden a crear conflictos de fusión, lo cual es una muy buena razón para que la mayoría de los mantenedores odien el reformateo solo por el simple hecho de hacerlo.
Jan Hudec
13

Puede extraer estos cambios solo si hay un problema abierto para corregir el formato. De lo contrario, inicie su propia rama, y ​​si el autor ve que más personas usan su rama simplemente porque es más legible. Se fusionarán en la rama por su cuenta, pero esté preparado para mantener su rama fusionando actualizaciones y arreglando constantemente el formato.

Si el proyecto no es lo suficientemente importante para usted como para mantener su propia sucursal, no valía la pena limpiarlo en primer lugar.

Las solicitudes de extracción pueden ser tomadas personalmente por el autor. No es un mecanismo para ofrecer críticas, y reformatear todo el código podría considerarse una crítica.

Si no desea mantener su propia sucursal, pero desea contribuir a un proyecto. Abra un nuevo problema y describa por qué el formato actual le está causando problemas, luego ofrezca resolver el problema para el autor. Si el autor está de acuerdo, le asignará el problema y ahora tiene permiso para realizar una solicitud de extracción.

Has tocado un tema que también estoy de acuerdo es un problema rampante en GitHub . Dejando a un lado el formato, hay una serie de proyectos que usan anotaciones incorrectamente y que causan estragos en muchos IDE. Puedo pensar en tres proyectos muy populares que usan indicadores desaprobados incorrectamente que propagan mensajes de advertencia en mi IDE. He enviado solicitudes de extracción para solucionarlos, pero los autores no usan el mismo IDE, por lo que se ignoran las solicitudes de extracción.

La ramificación, la fusión y la fijación parecen ser la única solución.

Reactgular
fuente
En su opinión, ¿consideraría que los beneficios para futuros contribuyentes son una "excusa" válida para los delitos de reparación de una solicitud de extracción? Por ejemplo, para que los futuros contribuyentes no tengan que preocuparse de mirar a través de toda la base de código para comprender su estilo de codificación.
raf
@RafalChmiel Cuando un autor acepta una solicitud de extracción, la persona que envió esa extracción se agrega al repositorio y se enumera públicamente como contribuyente al proyecto y permanece en la lista como contribuyente. Al revisar una solicitud de extracción, el autor tendrá esto en cuenta al decidir aceptarla. Tenga eso en cuenta al enviar solicitudes de extracción. Haz cosas dignas de ser un contribuyente.
Reactgular
¿Lo que quise decir es que beneficiar a los futuros contribuyentes mediante la reparación de delitos sería una razón válida para fusionar un RP con tales soluciones? ¿Por qué debería el mantenedor fusionar tal PR? Tal vez la redacción de mi pregunta estaba un poco fuera de lugar.
raf
2
@RafalChmiel todo lo que dices como razón es solo tu opinión. Si es un código ofensivo que parece un toro, entonces escribe tus miedos en un problema. Si el autor defiende contra ese tirón, limpie sus lágrimas con un pañuelo.
Reactgular
10

Tomado del sitio de Rubocop (énfasis mío):

Una cosa siempre me ha molestado como desarrollador de Ruby: los desarrolladores de Python tienen una excelente referencia de estilo de programación (PEP-8) y nunca obtuvimos una guía oficial que documente el estilo de codificación de Ruby y las mejores prácticas.

Por favor entiende:

No hay una guía oficial de estilo Ruby

No digo que las guías de estilo sean malas. Pero no solo no hay una guía oficial, sino que tener una guía de estilo es una elección que se realiza a nivel personal, de proyecto, de equipo y de empresa. Las guías de estilo son, para usar un término de psicología, una "norma social en el grupo".

¿Qué significa eso para ti? Bueno, si no se lo considera parte del grupo, eso significa que cualquier cosa que usted, o cualquier otro sitio web diga, es muy poco probable que tenga influencia sobre el grupo. Entonces, si no es un contribuyente activo y respetado para ese proyecto específico, lo más probable es que sus sugerencias sean ignoradas o, en el mejor de los casos, serán un recordatorio de consideraciones anteriores sobre tener una guía de estilo. En el peor de los casos, se tomará como un insulto, o como un intruso o un ciclista metiendo la nariz donde no pertenece.

¿No puedes sugerir una guía de estilo?

De hecho, eso parece ser lo que quieres hacer: crees en el valor de las guías de estilo, valoras mucho la coherencia y quieres evangelizar por la dedicación a las pautas de estilo unificadas.

Eso está bien, siempre y cuando sea realmente claro, de eso se trata y de lo que quiere lograr. Si crees en una Guía de Estilo en particular y crees que es La Guía de Estilo Verdadera, o al menos mejor que lo que sea que practiquen esos paganos sin ley, entonces está bien también.

Pero lo que la gente no aprecia es que le digan que su comportamiento no se ajusta a las reglas no oficiales, no vinculantes y en gran medida arbitrarias que provienen de una fuente que no consideran una autoridad legítima. Cuando eso es lo que se propone hacer, o si simplemente que es lo que se percibe a estar haciendo, obtendrá menos de la "alfombra roja", y más de los "nativos furiosos con lanzas y una olla grande de ebullición" tratamiento.

BrianH
fuente
3

Para ofrecer una opinión alternativa aquí, en muchos casos, tal cambio sería bienvenido. Los proyectos de código abierto tienden a tener muchos autores. A menudo no existe un "estilo de codificación"; el estilo es el que usó la persona que escribió el código en cuestión. Si un archivo fue escrito por una persona diferente a otra, el estilo también puede ser diferente. Incluso en proyectos donde hay un estilo de consenso, a menos que verifiquen este estilo regularmente, a menudo no se usa.

Un ejemplo común de esto en mi experiencia es cuando alguien contribuye con un código mediante solicitud de extracción que es de estilo de calidad relativamente baja. Sin embargo, el código puede funcionar. Diferentes personas tienen diferentes puntos de vista sobre esto. Algunas personas se negarán a fusionar una solicitud de extracción a menos que el estilo sea bueno. A algunas personas no les importa, siempre que el código funcione. Algunas personas prefieren tener un buen estilo, pero no quieren asustar a los contribuyentes con un montón de comentarios de "arreglar el espacio en blanco aquí" (personalmente siempre me siento un poco culpable haciendo estos comentarios, aunque sé que son para mejor bueno de la base de código, porque parece que podría asustar al contribuyente).

Así que no asumas directamente que el estilo que ves es el estilo que el proyecto quiere. De hecho, eso probablemente puede generalizarse para contribuir al código abierto en general: no asuma que el código que tiene un proyecto de código abierto es el código que quiere .

Sin embargo, debe tener en cuenta algunas cosas:

  • Algunas personas son religiosas sobre el estilo. Si queda claro que no quieren ceder, no se molesten.

  • Este es un gran problema de bicicletas . Todos y sus hermanos tienen una opinión sobre estas cosas. Debido a esto, obtener una solicitud de extracción combinada puede ser difícil.

  • También puede ser difícil fusionar una solicitud de extracción de este tipo porque generará conflictos de fusión muy rápidamente; básicamente cada vez que cualquier parte de la base de código que modificó cambia, incluso si es de manera trivial.

Me apegaría al enfoque de "preguntar primero". Si están abiertos a esto y usted está dispuesto a cumplir con la solicitud de extracción hasta su finalización, hágalo.

asmeurer
fuente
2

Solía ​​ser grande en tener una buena guía de estilo, pero dado el estado de las cosas en Ruby, "he seguido adelante".

Básicamente vivo con lo que estoy trabajando y de otra manera sigo las convenciones generales que aprendí de varios trabajos.

Para Ruby, que es mi idioma de elección, también he dividido (en mi cabeza) el estilo en universalmente aceptado, generalmente aceptado, mis preferencias y mejores prácticas. Cosas que son aceptadas universalmente Podría enviar un cambio como parte de una solicitud de cambio para un problema o solicitud de rama de función.

Ejemplos de cada estilo (en mi opinión):

Universalmente aceptado por Ruby:

  • espacios no pestañas
  • dos sangrías espaciales
  • method_names_use_underscores
  • Las constantes comienzan con mayúsculas.

Generalmente aceptado:

  • No use paréntesis si no es necesario
  • Úselo { }para bloques de una línea y do endpara bloques de varias líneas.
  • CONSTANTS_ARE_IN_ALL_CAPS
  • Use sentencias predicativas ( cond ? true : false) sobre if thensi la expresión se ajusta a 1 línea.

Preferencias personales:

  • Longitud de línea de 120 líneas de caracteres.
  • las whendeclaraciones de caso están sangradas 2 de la declaración de caso.
  • Use comillas simples a menos que se necesiten dobles.

Sin acuerdo:

  • Declaraciones de casos
  • Longitud de la línea

Mejores prácticas:

  • métodos pequeños, <= 5 líneas si es posible.
  • clases pequeñas, <= 100 líneas si es posible.
  • Evitar constantes
  • El código tiene pruebas

Finalmente, como otros han detallado, debe preguntar primero. Al final del día, la clave del estilo es la comunicación entre los desarrolladores y la sensibilidad hacia los demás. Por ejemplo, si quiero hacer un cambio de estilo en un proyecto de código abierto, a menudo haré una solicitud de extracción para una o dos características reales o correcciones de errores primero. Una vez que el mantenedor me conoce y ve que he contribuido, entonces podría sugerir cambios de estilo. Sin embargo, solo los sugiero . Por ejemplo, "Al hacer otra función en el proyecto x, noté que tienes una sangría de 4 espacios en un par de archivos y me preguntaba si podría cambiarlos a 2".

Michael Durrant
fuente
1

Aunque son pequeños y fáciles de arreglar, ¿cree que está bien cambiar el estilo de codificación de un proyecto de código abierto arreglando los delitos y haciendo una solicitud de extracción?

En resumen, no!

Por supuesto, estoy asumiendo aquí que estos son solo asuntos de estilo y que no son errores reales: las solicitudes de extracción para este último siempre serían sensatas (OMI). También estoy asumiendo que eres un extraño para el proyecto y no en contacto con las personas que ya lo mantienen.

Sin embargo, entre cualquier idioma siempre hay algunas personas que tienen sus preferencias que difieren de la guía de estilo, para bien o para mal, e intentan imponer esos cambios de estilo en personas que no conoces , y un proyecto del que no eres parte de si nada más podría parecer un poco grosero. Después de todo, ¿qué está logrando (en realidad) si la solicitud fue aceptada? Con toda probabilidad, si los miembros de un proyecto quisieran cambiar el estilo a algo diferente, ya lo habrían hecho, y todo lo que harían con esa solicitud es forzarles un estilo que no necesariamente funcione mejor para los miembros existentes.

Esto cambia ligeramente si está dispuesto a contribuir al proyecto de otras maneras que no sean "arreglos" de estilo. Diría que si desea abrir un diálogo con los encargados del mantenimiento sobre cómo le gustaría trabajar en el proyecto pero encuentra un estilo no estándar difícil, entonces está bien. ¡Pero realmente no iría a ciegas creando solicitudes de extracción para un montón de proyectos que solo contienen cambios de estilo!

berry120
fuente
1

CUALQUIER cambio puede introducir errores, incluso cambiar el formato tipográfico del código.
Por lo tanto, no se deben realizar cambios en el código a menos que haya un caso comercial válido, y "pero el código no se ve bien" o "el código no sigue los estándares de codificación" no es un caso comercial válido. El riesgo es simplemente demasiado grande.
Ahora, si de todos modos está realizando cambios importantes en un archivo fuente, puede ser aceptable alinear todo el archivo con los estándares, pero lo más probable es que realice pequeños cambios, en cuyo caso es casi universalmente preferible mantener sus propios cambios en alinearse con el código existente aunque ese código existente no siga los estándares de codificación.
Demonios, el código bien podría ser el resultado de un generador de código y, a veces, regenerarse. Los generadores de código son conocidos por producir código feo ...

jwenting
fuente
1

Tengo un par de proyectos .NET moderadamente exitosos, y he tenido un par de relaciones públicas de personas que parecen haber revisado el código con ReSharper y StyleCop y "arreglado" un montón de cosas. No acepto esas relaciones públicas, por un par de razones:

  • Si bien muchos de los cambios son para mejor, algunos de ellos impactan negativamente en el código de alguna manera, generalmente en el rendimiento, y elegir las partes buenas es demasiado trabajo.
  • Incluso si todos los cambios fueron benignos o incluso beneficiosos, cada cambio en cada archivo en cada confirmación debe revisarse, y nuevamente, solo toma demasiado tiempo.

Dicho esto, si alguien quisiera agregar una mejor verificación de errores o comentarios de documentos, aceptaría ese PR en un abrir y cerrar de ojos.

Mark Rendle
fuente
-1

Debe averiguar si los encargados del mantenimiento consideran que estas violaciones del estilo de codificación son un defecto o si el proyecto tiene un estándar diferente para el estilo de codificación.

Si tiene un estándar diferente, puede ofrecer ayuda para documentarlo o formalizarlo. Entonces podría resultar que hay algunas violaciones de ese estilo, y usted podría arreglarlas.

Tim
fuente
esto no parece ofrecer algo de valor sobre otra respuesta que fue publicada varias horas antes de éste
mosquito