¿Cómo manejar un TODO en una solicitud de extracción?

24

Cuando reviso los cambios en una solicitud de extracción, a veces me encuentro con un comentario con una nota "TODO" que puede estar allí por diferentes razones, en nuestro caso principalmente debido a:

  • La solución utilizada para resolver un problema puede mejorarse, pero requeriría una inversión de tiempo significativamente mayor. Un autor eligió una solución más rápida pero comentó que hay una opción mejor disponible
  • hay un código temporal para solucionar un error existente que debería corregirse pronto

Sabiendo que TODOgeneralmente permanecen en la base de código durante la vida útil de la base de código, ¿cómo debo reaccionar ante ellos en una solicitud de extracción? ¿Cómo puedo solicitar cortésmente evitarlo, o si está realmente justificado, cómo puedo asegurarme de que el autor del RP lo seguirá más adelante en el futuro?

alecxe
fuente
Si los TODO deshechos se están convirtiendo en un problema para su equipo, ¿no podría asignar a alguien (o, si carece de esa autoridad usted mismo, pedirle a su jefe que asigne a alguien) pasar algún tiempo revisando todo su código y haciendo todos los TODO?
nick012000
Un factor importante es si el TODO en cuestión es de una naturaleza que puede ser resuelta por desarrolladores que no sean el autor. Otro factor es evaluar pragmáticamente el riesgo o la relevancia de ese TODO: ¿es solo un lobo llorando?
rwong
Tener algunos TODOS en su base de código no es un problema en absoluto.
Oliver Watkins

Respuestas:

26

Cuando dice que "generalmente permanecen en la base de código durante la vida útil de la base de código" en su equipo / departamento / organización, considere lo siguiente:

  • Anótelo en su Departamento de Defensa que TODO, FIXMEo etiquetas similares deben ser evitados.
  • Use una herramienta de análisis de código estático como SonarQube para marcar automáticamente la construcción inestable.
  • Permítales temporalmente si, y solo si, hay un ticket correspondiente en su rastreador de problemas. Entonces, el código puede verse comoTODO [ID-123] Description ...

Como mencioné en mi comentario , la última declaración probablemente solo tiene sentido en un entorno que no permite que los tickets se pudran (por ejemplo, si sigue una política de cero errores ).

Personalmente, creo que TODOlas veces son razonables, pero no se deben usar en exceso. Tomado del "Código limpio: un manual de artesanía ágil de software" de Robert C. Martin (p. 59):

TODOs son trabajos que el programador cree que deberían hacerse, pero por alguna razón no puede hacer en este momento. Puede ser un recordatorio para eliminar una función obsoleta o una súplica para que otra persona vea un problema. Puede ser una solicitud para que otra persona piense en un nombre mejor o un recordatorio para hacer un cambio que depende de un evento planificado. Cualquier otra cosa que una TODOpodría ser, es no una excusa para salir mal código en el sistema.

beatngu13
fuente
2
¿El boleto correspondiente no permanecerá en la cartera de pedidos para siempre? He visto que TODOS y las entradas quedan sin resolver para siempre.
dzieciou
55
@dzieciou no necesariamente, pero, por supuesto, existe el riesgo de que también permanezca allí para siempre. Sin embargo, si tiene un ticket, ese riesgo es probablemente menor en comparación con un comentario de código solamente. Creo que depende del equipo / departamento / organización si esto tiene sentido.
13
Si tiene una política de no hacer todo, los desarrolladores simplemente dejarán el comentario de todo fuera del código y dejarán el código cuestionable rápido y sucio. Mala idea.
RibaldEddie
8
Todos son una forma de comunicación. Entre desarrolladores. A veces durante largos períodos de tiempo. El uso de la palabra TODO en un comentario de código no es más que azúcar sintáctica para una preocupación particular.
RibaldEddie
2
Creo que la mención de agregarlo al rastreador de problemas es clave aquí. Si haces eso, incluso podría suceder que la gente empiece a arreglarlo sin que tú lo sepas. Lo he visto suceder en una organización; los problemas se solucionaron solo porque a la gente le gustaba corregir errores, refactorizar código, etc. A veces puede ser bastante bueno.
Por Lundberg
5

TODOS ellos mismos no son malvados. Soy un gran fanático de nunca ir más allá de una capa de profundidad, y siempre soluciono 1 problema por boleto de rastreador.

A menudo uso TODO o FIXME como una forma de recordarme a mí mismo que necesitaba o quería volver para solucionar un problema.

Por ejemplo, puedo llamar a add (a, b) y agregar TODO para refactorizar el método add. No quiero trabajar en el método de agregar ahora, pero sí quiero volver a él.

Entonces, en una solicitud de extracción, verá TODO o FIXME. Utilizo FIXME, por ejemplo, para alertar a otros desarrolladores de áreas de código de las que tienen responsabilidad, con las que no debo meterme. Por ejemplo, FIXME add no puede aceptar números negativos.

Para evitar el problema de que no se vean o se ignoren, utilizo un script que enumera todas las líneas TODO FIXME y DEGUG. Y eso se agrega al mensaje de confirmación.

Es difícil ignorar un mensaje de confirmación de 400 líneas que es TODOS. Entonces, las personas los arreglan, ya sea mientras tocan el código en cuestión o creando nuevos tickets y arreglando el código del problema de forma independiente.

Para ser justos, también me aseguro de que cada proyecto tenga tiempo de limpieza de código incorporado. Sí, puede estar listo para lanzarse el 15, pero estábamos limpiando el código del 15 al 30. (parece extraño, pero si alguna vez ha administrado un producto, sabe que si intenta tener tareas de baja visibilidad antes del lanzamiento, nunca se le permitirá llegar a ellas. Algo más tendrá prioridad).

coteyr
fuente
1
Estoy de acuerdo en que los TODOs no son malvados per se, pero usarlos a menudo es lo que consideraría ruido. También creo que un mensaje de confirmación de 400 líneas se ignora fácilmente porque las personas tienden a omitir tanto texto. Además, muchas interfaces Git / VCS (por ejemplo, GitHub) truncan cualquier línea de asunto por más tiempo que un cierto número de caracteres.
beatngu13
Sí, ese es mi punto, en alrededor de 4-5 líneas, las personas tienden a querer limpiarlo. Entonces comienzan a hacer TODO. 400 líneas fue una exageración.
coteyr
Me interesaría cómo funciona el script para agregar TODOS al mensaje de confirmación.
Simon
Hay un enlace "commit-msg" con el que puedes vincular.
coteyr
1

Sucederá que hay solicitudes de extracción que no son perfectas. En este momento estoy haciendo un trabajo que se puede hacer "lo suficientemente bueno" en el tiempo disponible, pero no perfecto. Entonces, envío una solicitud de extracción, pongo TODO con comentarios en los lugares correctos del código y, al mismo tiempo, agrego otra solicitud de cambio para cambiar las cosas de "lo suficientemente bueno" a "perfecto".

Esta nueva solicitud de cambio se puede priorizar y se manejará cuando sea el elemento de mayor prioridad. Si se decide que necesita ser perfecto en este momento , entonces será lo próximo que se entregue.

Ahora su pregunta: "¿Cómo puedo solicitar cortésmente evitarlo, o si está realmente justificado, cómo puedo asegurarme de que el autor del RP lo seguirá más adelante en el futuro?" Con lo que describo, eso parece ser bastante tonto. Si tengo la opción entre enviar tarde y enviar lo que es lo suficientemente bueno, entonces no es su decisión. Puede preguntarle a su gerente de producto si lo desea. ¿Y "asegurarse de que el autor del PR lo seguiría"? Si tiene un equipo, entonces debe asegurarse de que esté registrado en sus sistemas y, con suerte, su equipo esté lo suficientemente bien organizado como para que las cosas registradas no se pierdan, y alguien lo arreglará eventualmente, cuando no haya elementos de mayor prioridad. Recuerde, es un esfuerzo de equipo.

gnasher729
fuente
1

Si su proyecto rastrea elementos pendientes en el código fuente con TODOcomentarios, entonces debe permitirlo.

El hecho de que la presencia de un TODOcomentario en la solicitud de extracción sea molesta, debería decirle que rastrear los elementos pendientes en el código fuente es una mala idea. Las cosas tienden a perderse o ignorarse de esa manera. Ahora, si está hablando de una solicitud de extracción a una "bifurcación de trabajo", la situación es diferente. Una "bifurcación de trabajo" es solo eso: un trabajo en progreso. Pero un tenedor como ese generalmente no requiere una solicitud de extracción. Las "Reglas de la casa" sugeridas aquí son para solicitudes de extracción para la rama maestra .

Regla de la casa n. ° 1: todos los compromisos con el maestro deben estar listos para la prueba, ya que el maestro se construye diariamente después de cualquier registro. Esas confirmaciones también deben incluir cualquier prueba adicional requerida.

Si el TODOcomentario está allí porque el código no está terminado, o las pruebas no están terminadas, o el código no está listo para probar, entonces ese código pertenece a una confirmación local, no a una solicitud de extracción. Llámame cuando esté listo.

Regla de la casa n. ° 2: toda la información sobre los problemas abiertos se almacena en el rastreador de problemas. Todo ello. Notas, garabatos, corazonadas, lo que sea.

Si el TODOcomentario se refiere a un problema abierto y no es una solución real para ese problema, entonces esa información pertenece al rastreador de problemas. De esa manera, antes de que se cierre un problema, toda la información se puede revisar y verificar, si es necesario, para asegurarse de que el problema se resuelva realmente.

Regla de la casa # 3: toda la información relacionada con las tareas pendientes del proyecto pertenece a la cola de prioridad (o cualquiera que sea el nombre de su sistema).

Para aclarar, una tarea de proyecto pendiente es algo que debe hacerse en el proyecto que tiene una prioridad establecida, ya sea un defecto que se descubrió antes de generar un ticket de problema, o la implementación de un requisito de diseño específico, o uno de componentes necesarios de ese requisito.

Si el TODOcomentario está ahí para decir que el nuevo código tendrá un impacto en una tarea pendiente, o para señalar algo más en la base de código que se debe observar que se descubrió al implementar el nuevo código, entonces esa información pertenece a la cola de prioridad, ya sea en la tarea existente o como una nueva.

Regla de la casa # 4 - Las sugerencias pertenecen al Cuadro de ideas (o lo que sea).

Si el TODOcomentario sugiere un cambio en el diseño o la implementación del software, entonces esa información pertenece al cuadro de ideas del proyecto, o "vNext", o "Design Notes", o lo que sea que tenga para ese tipo de cosas.

Regla de la casa # 5: los TODOcomentarios no están permitidos en el código fuente. PERÍODO.

Si se apega a esta regla, no tendrá que preocuparse de que alguien haga un seguimiento de nada.

Mark Benningfield
fuente