¿Es una mala práctica no eliminar archivos redundantes de inmediato de VCS sino marcarlos como "Para ser eliminados" con comentarios primero?

53

Quería saber si la forma en que trato con los archivos fuente que necesitan ser eliminados del control de versiones podría considerarse una mala práctica.

Quiero explicártelo basado en ese ejemplo:

Recientemente me enojé mucho porque tuve que ordenar tediosamente las clases de Java en un programa que básicamente era un código muerto, sin embargo, no estaba documentado en ninguna parte y tampoco fue comentado en esas clases de Java. Por supuesto, tenían que eliminarse, pero antes de eliminar cosas tan redundantes tengo un hábito, algunos pueden decir extraño:

No elimino dichos archivos redundantes de inmediato a través de SVN-> Eliminar (reemplace con el comando Eliminar del sistema de control de versiones de su elección), sino que pongo comentarios en esos archivos (me refiero tanto al principio como al pie de página) que van a se elimine + mi nombre + la fecha y también, lo que es más importante, POR QUÉ SE ELIMINAN (en mi caso, porque estaban muertos, código confuso). Luego los guardo y los confirmo para el control de versiones. La próxima vez que tenga que confirmar / registrar algo en el proyecto para el control de versiones, presiono SVN-> Eliminar y luego finalmente se eliminan en Control de versiones, aunque, por supuesto, aún se pueden restaurar a través de revisiones y es por eso que adopté ese hábito.

¿Por qué hacer esto en lugar de eliminarlos de inmediato?

Mi razón es que quiero tener marcadores explícitos al menos en la última revisión en la que existían esos archivos redundantes, por qué merecían ser eliminados. Si los elimino de inmediato, se eliminan pero no se documenta en ninguna parte por qué se eliminaron. Quiero evitar un escenario típico como este:

"Hmm ... ¿por qué se eliminaron esos archivos? Trabajé bien antes". (Presiona 'revertir' -> el tipo que revertió se fue para siempre o no está disponible en las próximas semanas y el próximo cesionario tiene que averiguar tediosamente como yo de qué tratan esos archivos)

Pero, ¿no notas por qué esos archivos se eliminaron en los mensajes de confirmación?

Por supuesto que sí, pero a veces los colegas no leen un mensaje de confirmación. No es una situación típica que cuando intente comprender el código (en mi caso muerto) que primero verifique el registro de control de versiones con todos los mensajes de confirmación asociados. En lugar de rastrear el registro, un colega puede ver de inmediato que este archivo es inútil. Le ahorra tiempo y él / ella sabe que este archivo probablemente fue restaurado para mal (o al menos plantea una pregunta.

Bruder Lustig
fuente
120
Básicamente, está intentando replicar el trabajo de su sistema de control de versiones directamente en el archivo. Hacer eso definitivamente elevaría una bandera en mi cabeza. Si sus colegas no leen los mensajes de confirmación y resucitan un archivo que se eliminó correctamente y pasa la revisión del código, definitivamente hay algo mal en su equipo, y es una gran oportunidad para enseñarles mejor.
Vincent Savard
66
@ GregBurghardt Esto parece una buena pregunta sobre una mala idea. ¿Quizás las personas están votando en base a la segunda parte de eso (cuando, en mi opinión, deberían estar votando por la primera)?
Ben Aaronson
13
"La próxima vez que tenga que confirmar / registrar algo en el proyecto para controlar la versión, presiono SVN-> Eliminar" ¿Estás diciendo que los eliminas en una confirmación (potencialmente) totalmente no relacionada?
Kevin
21
Por supuesto que sí, pero a veces los colegas no leen un mensaje de confirmación. - si no están revisando el mensaje de confirmación, es seguro asumir que no están interesados ​​en por qué se eliminó el archivo ... de lo contrario, deben verificar el mensaje de confirmación.
Ant P
99
Si quiero saber por qué desapareció un archivo de mi pago de VCS, primero buscaré en el historial de cambios , y si eso no explica las cosas, leeré la discusión que ocurrió cuando se revisó la eliminación. (Si no tiene un proceso de revisión de código por el que pasa cada cambio, tiene problemas más grandes). Y si eso aún no es esclarecedor, hablaré con quien haya eliminado el archivo. Podría mirar el contenido anterior del archivo para tratar de descubrir lo que solía hacer, pero no para obtener una explicación de la eliminación.
zwol

Respuestas:

110

El problema con agregar un comentario a un archivo que debería eliminarse, en lugar de eliminarlo en el control de origen y poner la explicación allí, es la suposición de que si los desarrolladores no leen los mensajes de confirmación seguramente leerán los comentarios en el código fuente.

Desde la perspectiva de un extraño, esta metodología parece estar enraizada en una visión muy conservadora del control de la fuente.

"¿Qué pasa si elimino este archivo no utilizado y luego alguien lo necesita?" Alguien podría preguntar.

Estás utilizando el control de fuente. Revierta el cambio o, mejor aún, hable con la persona que eliminó el archivo (comuníquese).

"¿Qué pasa si elimino el archivo muerto, entonces alguien comienza a usarlo nuevamente y hacen cambios?" alguien más podría preguntar.

De nuevo, está utilizando el control de fuente. Obtendrá un conflicto de fusión que una persona debe resolver. La respuesta aquí, como con la última pregunta, es comunicarse con sus compañeros de equipo.

Si realmente duda de que un archivo deba eliminarse, comuníquese antes de eliminarlo del control de origen. Tal vez solo dejó de usarse recientemente, pero una característica próxima podría requerirlo. No lo sabes, pero uno de los otros desarrolladores podría.

Si se debe quitar, retírelo. Cortar la grasa de la base del código.

Si realizó un "oopsie" y realmente necesita el archivo, recuerde que está utilizando el control de código fuente para poder recuperar el archivo.

Vincent Savard, en un comentario sobre la pregunta, dijo:

... Si sus colegas no leen los mensajes de confirmación y resucitan un archivo que se eliminó correctamente y pasa la revisión del código, definitivamente hay algo mal en su equipo, y es una gran oportunidad para enseñarles mejor.

Este es un buen consejo. Las revisiones de código deberían estar atrapando este tipo de cosas. Los desarrolladores deben consultar los mensajes de confirmación cuando se realiza un cambio inesperado en un archivo, o se elimina o cambia el nombre de un archivo.

Si los mensajes de confirmación no cuentan la historia, entonces los desarrolladores también deben escribir mejores mensajes de confirmación.

Tener miedo de eliminar código o eliminar archivos es indicativo de un problema sistémico más profundo con el proceso:

  • Falta de revisiones de código precisas
  • Falta de comprensión sobre cómo funciona el control de fuente
  • Falta de comunicación del equipo.
  • Mensajes de confirmación deficientes por parte de los desarrolladores

Estos son los problemas a resolver, por lo que no siente que está tirando piedras en una casa de cristal cuando elimina código o archivos.

Greg Burghardt
fuente
3
"se supone que si los desarrolladores no leen los mensajes de confirmación seguramente leerán los comentarios en el código fuente" Creo que esta es una suposición bastante buena. Un desarrollador que cambia un archivo necesita realmente mirar el archivo para realizar la tarea, mientras que no hay nada que lo obligue a mirar el historial de control de origen.
AShelly
77
@AShelly: una tarea de desarrollador rara vez es cambiar un comentario. Las tareas implican cambiar el código, que es en lo que se enfoca el desarrollador, no en los comentarios.
Greg Burghardt
21
@AShelly El hecho de que tengan que abrir un archivo no significa que leerán un comentario en particular en la parte superior. El público objetivo para este cambio es el tipo de desarrollador más ajeno, el tipo que piensa que sus necesidades son las únicas necesidades y todo debería girar en torno a ellas. Es poco probable que lean tu comentario cortés. Peor aún, es probable que lo lean y luego simplemente vuelvan a la siguiente versión más antigua.
Cort Ammon
55
@AShelly: como ejemplo, normalmente abro archivos en una ubicación. En VS puedo abrir directamente a un método usando los menús desplegables en la parte superior o el menú contextual "ir a la definición". Nunca vería la parte superior del archivo. También en Sublime Text, frecuentemente abro una línea. Su diálogo abierto users.rb: 123 abriría users.rb directamente a la línea 123. Muchos editores de código tienen opciones muy similares.
coteyr
2
Además de lo anterior, cuanto más complejo es realizar tareas de limpieza como esta, es menos probable que las personas las realicen regularmente. Si puede salirse con la suya de manera más simple, disminuye la "energía de activación" para usted y para todos los demás. Esto es especialmente importante si está tratando de alentar un cambio de práctica para otros miembros del equipo. Cuanto más complejo es el procedimiento, más difícil es conseguir la aceptación.
xdhmoore
105

Sí, es una mala práctica.

Debe poner la explicación de la eliminación en el mensaje de confirmación cuando confirme la eliminación de los archivos.

Los comentarios en los archivos de origen deben explicar el código como se ve actualmente . Los mensajes de confirmación deben explicar por qué se hicieron los cambios en la confirmación, por lo que el historial de confirmación en el archivo explica su historial.

Escribir comentarios que expliquen los cambios directamente en la fuente misma hará que el código sea mucho más difícil de seguir. Piénselo: si comenta en el archivo cada vez que cambia o elimina el código, pronto los archivos se inundarán de comentarios sobre el historial de cambios.

JacquesB
fuente
66
Quizás agregue la respuesta a la pregunta: ¿es una mala práctica? Sí, porque ...
Juan Carlos Coto
1
A veces hago lo mismo (como OP) porque es más fácil descomentar que revertir. En casos raros, incluso si el código compila y pasa las pruebas automáticas, hay una razón para la existencia de ese código que pasé por alto que el probador manual detectaría. En este raro escenario, en lugar de pasar por un gran dolor o revertir varias confirmaciones más tarde, simplemente descomento el código (y reviso cómo se puede mejorar)
Andrew Savinykh
2
@AndrewSavinykh Eso es diferente, estás comentando código que todavía no estás seguro de querer eliminar.
Goyo
66
@AndrewSavinykh "dolor mayor o revertir cosas varias veces más tarde" - Me pregunto qué control de versión usas; Esto no debería ser un gran dolor. Ciertamente no está en Git, al menos no después de que lo hayas hecho un par de veces y hayas aprendido a qué estar atento ... y siempre que tu historial no esté plagado de compromisos innecesarios de ida y vuelta que te dan conflictos cada otra fusión. Y se compromete a que simplemente comentario algo fuera son bastante propensos a hacer esto ...
leftaroundabout
44
@AndrewSavinykh sí, y no es que no haya experimentado estos problemas: proyectos cuyos encargados nunca trabajaron realmente con el control de versiones y pusieron mucha información en los comentarios que realmente deberían haber sido mensajes de confirmación, en lugar de eso agregaron nuevas confirmaciones manuales de deshacer de revertir adecuadamente, etc. El estado resultante del repositorio resultó en mucha diversión de conflicto en cada rebase más grande ... Pero, y ese es mi punto, estos problemas a menudo son causados ​​en gran medida por soluciones como la que defiende aquí.
Leftaroundabout
15

En mi opinión, ambas opciones no son la mejor práctica , a diferencia de la mala práctica:

  1. Agregar comentarios solo agrega valor si alguien lee esos comentarios.
  2. Simplemente eliminar del VCS (con una razón en la descripción del cambio) puede tener un impacto crítico y muchas personas no leen la descripción del cambio, especialmente cuando están bajo presión.

Mi práctica preferida, en gran parte debido a que me han mordido varias veces a lo largo de los años, teniendo en cuenta que no siempre sabes dónde o quién usa tu código , es:

  1. Agregue un comentario que indique que es un candidato para la eliminación y una desaprobación #warning o similar, (la mayoría de los idiomas tienen algún tipo de mecanismo, por ejemplo, en Java , en el peor de los casos printserá suficiente una declaración o similar), idealmente con algún tipo de escala de tiempo y con datos de contacto. Esto alertará a cualquiera que todavía esté usando el código. Estas advertencias normalmente están dentro de cada función o clase si tales cosas existen .
  2. Después de un tiempo, actualice la advertencia a un alcance de archivo estándar #warning(algunas personas ignoran las advertencias de desaprobación y algunas cadenas de herramientas no muestran dichas advertencias por defecto).
  3. Más tarde, reemplace el alcance del archivo #warningcon uno #erroro equivalente: esto romperá el código en el momento de la compilación, pero puede, si es absolutamente necesario, eliminarse, pero la persona que lo elimina no puede ignorarlo. (Algunos desarrolladores no leen ni abordan ninguna advertencia o tienen tantas que no pueden separar las importantes).
  4. Finalmente marque los archivos (en la fecha de vencimiento o después), como eliminados en el VCS.
  5. Si elimina un paquete / biblioteca / etc. completo en la etapa de advertencia, agregaría un archivo README.txt o README.html o similar con la información sobre cuándo y por qué se planea deshacerse del paquete y dejar solo ese archivo, con un cambie para decir cuándo se eliminó, como el único contenido del paquete durante algún tiempo después de eliminar el resto del contenido.

Una ventaja de este enfoque es que algunos sistemas de versiones (CVS, PVCS, etc.) no eliminarán un archivo existente al finalizar la compra si se ha eliminado en el VCS. También les da a los desarrolladores concienzudos, aquellos que arreglan todas sus advertencias, mucho tiempo para abordar el problema o apelar la eliminación. También obliga a los desarrolladores restantes a al menos mirar el aviso de eliminación y quejarse mucho .

Tenga en cuenta que #warning/ #errores un mecanismo C / C ++ que causa una advertencia / error seguido del texto proporcionado cuando el compilador encuentra ese código, java / java-doc tiene una @Deprecatedanotación para emitir una advertencia sobre el uso y @deprecatedproporcionar alguna información, etc. Recetas para hacer similar en Python y he visto que se assertutiliza para proporcionar información similar a #errorla del mundo real.

Steve Barnes
fuente
77
En particular, dado que esta pregunta menciona Java, use el @deprecatedcomentario JavaDoc y la @Deprecatedanotación .
200_success
8
Esto parece más apropiado cuando quieres deshacerte de algo que todavía está en uso. La advertencia puede permanecer hasta que todos los usos hayan desaparecido, pero una vez que el código esté inactivo, debe eliminarse inmediatamente.
Andy
66
@Andy Uno de los problemas con el código de tipo de biblioteca, especialmente en Open Source o en grandes organizaciones, es que puedes pensar que el código está muerto pero descubres que está en uso activo en uno o más lugares que nunca imaginaste personalmente que estaba en uso. A veces, el código debe eliminarse, porque es realmente malo o incluye riesgos de seguridad, pero simplemente no sabe si y dónde se está utilizando.
Steve Barnes
1
@ 200_success gracias - mi experiencia en C / C ++ mostrando - agregado a la respuesta.
Steve Barnes
1
@SteveBarnes Quizás, pero eso no es lo que pregunta el OP. Ha verificado que el código está muerto.
Andy
3

Sí, diría que es una mala práctica, pero no porque esté en los archivos versus en el mensaje de confirmación. El problema es que está intentando realizar un cambio sin comunicarse con su equipo.

Cada vez que realice algún cambio en el enlace troncal, agregando, eliminando o modificando archivos de cualquier manera, debe, antes de volver a ingresar al enlace troncal, hablar sobre esos cambios directamente con un miembro (o miembros) del equipo que estaría directamente conocedores de ellos (esencialmente, discuta lo que pondría en esos encabezados directamente con sus compañeros de equipo). Esto asegurará que (1) el código que está eliminando realmente necesite ser eliminado, y que (2) su equipo sea mucho más probable que sepa que el código se elimina y, por lo tanto, no intente revertir sus eliminaciones. Sin mencionar que también obtendrá detección de errores y similares para el código que agregue.

Por supuesto, cuando cambias cosas importantes, también debes incluirlo en el mensaje de confirmación, pero como ya lo has discutido, no es necesario que sea la fuente de anuncios importantes. La respuesta de Steve Barnes también aborda algunas buenas estrategias para usar si el grupo potencial de usuarios para su código es demasiado grande (por ejemplo, usar los marcadores obsoletos de su idioma en lugar de eliminar los archivos al principio).

Si no desea pasar tanto tiempo sin comprometerse (es decir, su cambio tiene más sentido como varias revisiones), es una buena idea crear una rama desde el tronco y comprometerse con la rama, luego fusionar la rama nuevamente en el tronco cuando el El cambio está listo. De esa manera, el VCS todavía está haciendo una copia de seguridad del archivo, pero sus cambios no están afectando al tronco de ninguna manera.

TheHansinator
fuente
1

Un problema relacionado de proyectos que se basan en múltiples plataformas y configuraciones. El hecho de que determine que está muerto no significa que sea una determinación correcta. Tenga en cuenta que muerto en uso aún podría significar dependencia vestigial que aún debe ser eliminada, y algunos pueden haberse perdido.

Entonces (en un proyecto C ++) agregué

#error this is not as dead as I thought it was

Y lo comprobé. Espere a que se complete la reconstrucción de todas las plataformas normales y que nadie se queje de ello. Si alguien lo encontrara, sería fácil eliminar una línea en lugar de estar desconcertado con un archivo faltante.

En principio, estoy de acuerdo en que los comentarios que menciona duplican la función que debe hacerse en el sistema de gestión de versiones . Pero, puede tener razones específicas para complementar eso. No conocer la herramienta VCS no es una de ellas.

JDługosz
fuente
-1

En el continuo de malas prácticas, esto es bastante menor. Lo haré ocasionalmente, cuando quiera revisar una sucursal y poder ver el código anterior. Esto puede facilitar la comparación con el código de reemplazo. Por lo general, recibo un comentario de revisión de código "cruft". Con una pequeña explicación paso la revisión del código.

Pero este código no debería vivir por mucho tiempo. Generalmente elimino al comienzo del próximo sprint.

Si tiene compañeros de trabajo que no leen los mensajes de confirmación o no le preguntan por qué dejó el código en el proyecto, entonces su problema no es uno de mal estilo de codificación. Tienes un problema diferente.

Tony Ennis
fuente
Esto no parece ofrecer nada sustancial sobre los puntos hechos y explicados en las 6 respuestas anteriores
mosquito
-3

También puede refactorizar la clase y cambiarle el nombre con un prefijo UNUSED_Classname antes de realizar su truco. Entonces también debe confirmar directamente la operación de eliminación

  • Paso 1: renombra la clase, agrega tu comentario, confirma
  • Paso 2: elimine el archivo y confirme

Si es un código muerto, elimínelo. Cualquiera lo necesita, pueden regresar, pero el paso de cambio de nombre les impedirá usarlo directamente sin pensar.

También enseñe a las personas cómo ver todos los mensajes de confirmación para un archivo, algunas personas ni siquiera saben que es posible.

usuario275398
fuente
66
"Refactorizar" realmente no significa lo que crees que hace
Nik Kyriakides
66
No es necesario cambiar el nombre de la clase / método para garantizar que no se use, ya que eliminarlo funciona igual de bien. En cambio, el procedimiento es el mismo que cualquier otro cambio: 1) eliminar el código muerto , 2) ejecutar las pruebas , 3) si pasan, comprometerse con comentarios .
Schwern
1
@ user275398 También creo que renombrar los archivos es realmente demasiado. Y también desde el punto de vista tecnológico, SVN trata el cambio de nombre como si el archivo se eliminara y se creara con un nuevo nombre, por lo tanto, tiene una interrupción en el historial. Si restaura el archivo renombrado y mira el registro SVN del mismo, el historial anterior al cambio de nombre no está disponible. Para esto, debe revertir el archivo con el nombre anterior. Refactorizar es, por cierto, otra cosa, refactorizar es organizar el código existente en una nueva estructura.
Bruder Lustig
@BruderLustig: Un buen software ciertamente no hará eso. Git tiene git mv, que rastrea la historia. Mercurial también tiene hg mv. ¿Parece que también svn movedebería funcionar para SVN?
wchargin
@wchargin No, git mvsolo mueve el archivo y realiza una eliminación y creación de un archivo. Todo el seguimiento de movimiento ocurre cuando corres git log --followy similar. gitno tiene forma de rastrear los nombres, en realidad no rastrea los cambios en absoluto; en su lugar, realiza un seguimiento de los estados en el momento de la confirmación y descubre qué ha cambiado en el momento en que inspecciona el historial.
maaartinus