Prácticas ágiles: Revisión de código - ¿Falla la revisión o plantea un problema?

  

¿existen problemas o consideraciones inherentes con la obtención de un ticket de la parte posterior de una revisión, en lugar de fallarlo?

No inherentemente. Por ejemplo, la implementación del cambio actual puede haber desenterrado un problema que ya estaba allí, pero que no se conocía / aparecía hasta ahora. Fallar en el boleto sería injusto, ya que fallaría por algo no relacionado con la tarea realmente descrita.

  

en la revisión descubrimos una función

Sin embargo, supongo que la función aquí es algo que se agregó con el cambio actual. En este caso, el boleto debería fallar ya que el código no pasó la prueba de olfato.

¿Dónde dibujarías la línea, si no donde ya la dibujaste? Claramente, no cree que este código sea lo suficientemente limpio como para permanecer en el código base en su forma actual; Entonces, ¿por qué consideras dar un pase al boleto?

  

Falla la revisión del código, para que el ticket no se cierre en este sprint, y recibimos un pequeño golpe en la moral, porque no podemos pasar el ticket.

Me parece que estás argumentando indirectamente que estás tratando de darle a este boleto un pase para beneficiar la moral del equipo, en lugar de beneficiar la calidad del código base.

Si ese es el caso, entonces tienes tus prioridades mezcladas. El estándar de código limpio no debe modificarse simplemente porque hace más feliz al equipo. La corrección y limpieza del código no dependen del estado de ánimo del equipo.

  

El refactor es una pequeña pieza de trabajo, y se haría en el siguiente sprint (o incluso antes de que comience) como una pequeña historia de medio punto.

Si la implementación del ticket original causó el olor del código, entonces debe abordarse en el ticket original. Solo debe crear un nuevo ticket si el olor del código no se puede atribuir directamente al ticket original (por ejemplo, un escenario "de paja que rompió la espalda del camello").

  

Los recursos que puedo encontrar y que he leído las revisiones de los códigos de detalle son 100% o nada, por lo general, pero encuentro que generalmente no es realista.

Pasar / fallar es inherentemente un estado binario , que es inherentemente todo o nada.

A lo que te refieres aquí, creo, es más que interpretar que las revisiones de código requieren un código perfecto o que de lo contrario fallan, y ese no es el caso.

El código no debe estar inmaculado, simplemente debe cumplir con el nivel razonable de limpieza que emplea su equipo / empresa. La adhesión a ese estándar es una opción binaria: se adhiere (pasa) o no (falla).

Según su descripción del problema, está claro que no cree que esto se adhiera al estándar de código esperado y, por lo tanto, no se debe aprobar por razones ulteriores, como la moral del equipo.

  

De lo contrario, la tarea se ajusta a la definición de hecho.

Si "hace el trabajo" fue el mejor punto de referencia para la calidad del código, entonces no habríamos tenido que inventar el principio de código limpio y buenas prácticas para comenzar: el compilador y las pruebas de unidad ya serían nuestra automatización. revisa el proceso y no necesitarás revisiones de código ni argumentos de estilo.

  

Al final de un sprint de 2 semanas y una tarea tiene una revisión de código [...] Trabajo de refactorización sencilla.

¿Por qué aparece eso al final del sprint? Una revisión del código debería ocurrir tan pronto como creas que el código está terminado (o incluso antes). Debes verificar tu definición de hecho con cada historia que termines.

Si se encuentra terminando historias tan poco antes de la revisión de su demostración / sprint que no puede realizar una tarea "pequeña", entonces necesita mejorar su estimación de su trabajo. Sí, esa historia no se terminó. No debido a una revisión del código, sino porque no planeaba incorporar cambios de la revisión del código. Eso es como estimar "pruebas" para tomar tiempo cero, porque "si lo programó correctamente, simplemente funcionará, ¿verdad?". Esa no es la forma en que funciona. Las pruebas encontrarán errores y la revisión del código encontrará cosas para cambiar. Si no lo fuera, sería una gran pérdida de tiempo.

Para resumir: sí, el DoD es binario. Aprobar o suspender. Una revisión de código no es binaria, debería ser más como una tarea en curso. No puedes fallar . Es un proceso y al final está hecho. Pero si no planificas adecuadamente, no llegarás a esa etapa de "hecho" a tiempo y estarás atrapado en un territorio de "no hecho" al final del sprint. Eso no es bueno para la moral, pero debe tenerlo en cuenta en la planificación.

Simple: revisa el cambio . No revisa el estado del programa de lo contrario. Si soluciono un error en una función de 3,000 líneas, entonces verificas que mis cambios corrijan el error, y eso es todo. Y si mi cambio corrige el error, acepta el cambio.

Si piensa que la función es demasiado larga, entonces ingresa una solicitud de cambio para acortarla o la divide, después de mi cambio fue aceptado, y esa solicitud de cambio puede ser Priorizado según su importancia. Si se toma la decisión de que el equipo tiene cosas más importantes que hacer, entonces se manejará más adelante.

Sería ridículo si pudiera decidir las prioridades de desarrollo durante una revisión del código, y rechazar mi cambio por ese motivo sería un intento de decidir las prioridades de desarrollo.

En resumen, es absolutamente aceptable aceptar un cambio de código e inmediatamente generar un ticket según las cosas que viste al revisar el cambio. En algunos casos, incluso lo hará si el cambio mismo causó los problemas: si es más importante tener los cambios ahora que corregir los problemas. Por ejemplo, si otros fueron bloqueados, esperando el cambio, entonces desea desbloquearlos mientras se puede mejorar el código.

  

Falla la revisión del código, para que el ticket no se cierre en este sprint, y recibimos un pequeño golpe en la moral, porque no podemos pasar el ticket.

Este parece ser el problema.
En teoría, usted sabe lo que debe hacer, pero está cerca de la fecha límite, así que no quiere hacer lo que sabe que debe hacer.

La respuesta es simple: haz lo que harías si obtuvieras el mismo código para revisar el código el primer día del sprint. Si sería aceptable, entonces debería hacerlo. Si no lo fuera, no lo haría ahora.

Una gran parte del proceso es decidir qué significa hecho y apegarse a sus armas. También significa no comprometerse en exceso y hacer que las revisiones de sus colegas se realicen a tiempo para permitir que las pruebas garanticen que el trabajo también está funcionalmente completo.

Cuando se trata de problemas de revisión de código, hay algunas maneras de manejarlo y la elección correcta depende de algunos factores.

• Puede limpiar el código usted mismo y dejar que la persona sepa lo que hizo. Proporciona algunas oportunidades de tutoría, pero esto debería ser algo bastante simple que se puede hacer en unos minutos.
• Puedes devolverlo con comentarios sobre lo que está mal. Si el manejo de errores se realiza de manera deficiente, o el desarrollador sigue repitiendo los mismos errores, esto puede justificarse.
• Puede crear un ticket e incurrir en deudas técnicas. El boleto está allí para asegurarse de que lo pague más tarde. Puede ser que estés en un momento crítico y en el proceso de revisar los cambios, veas un problema mayor que no esté directamente relacionado con el cambio.

La conclusión es que cuando haya terminado con el trabajo debe hacerlo. Si hay problemas más grandes de los que trabajó el desarrollador, levante la bandera y continúe. Pero no debería estar en una posición en la que haya horas antes del final del sprint y justo ahora esté llegando a la revisión por pares. Eso huele a sobredecometer sus recursos, o postergar las revisiones por pares. (un olor del proceso).

No hay problemas inherentes con la eliminación de prioridades de los problemas de revisión de código, pero parece que los principales problemas que debe acordar, como equipo, son:

1. ¿Cuál es el propósito de la revisión de su código?
2. ¿Cómo se relacionan los resultados de la revisión del código con la Definición de Hecho para un elemento de trabajo?
3. Si la revisión del código se aplica como una prueba de selección de nivel, ¿qué problemas se consideran "bloqueadores"?

Todo esto se reduce a lo que el equipo ha aceptado como la definición de Hecho. Si pasar la revisión de código con Cero problemas es la definición de hecho para un elemento de trabajo, entonces no puede cerrar un elemento que no haya cumplido con este requisito.

Es lo mismo que si durante una prueba unitaria fallara una prueba unitaria. Corrigiría el error, no ignoraría la prueba de la unidad, si pasar las pruebas de la unidad era un requisito para terminar.

Si el equipo no ha aceptado que las revisiones de código sean una definición de Hecho, entonces sus revisiones de código no son una prueba de aceptación del elemento de trabajo. Son una actividad de equipo que forma parte de su proceso de acumulación para buscar trabajo adicional que pueda ser necesario. En ese caso, cualquier problema que descubra no está relacionado con los requisitos del elemento de trabajo original y son nuevos elementos de trabajo para que el equipo priorice.

Por ejemplo, podría ser completamente aceptable que un equipo no priorice la corrección de errores tipográficos en algunos nombres de variables, ya que no afecta la funcionalidad empresarial que se ha entregado, aunque el equipo realmente odia ver el nombre de la variable "myObkect".

Las respuestas más votadas aquí son muy buenas; éste aborda el ángulo de refactorización.

En la mayoría de los casos, la mayoría del trabajo cuando la refactorización comprende el código existente; cambiarlo después es generalmente la parte más pequeña del trabajo por una de dos razones:

1. Si el código es más claro y conciso, los cambios necesarios son obvios. A menudo, comprendió el código al probar cambios que parecían más limpios y ver si realmente funcionaron, o si se perdió alguna sutileza en el código más complejo.

2. Ya tiene en mente un diseño o estructura particular que necesita para facilitar la creación de una nueva función. En ese caso, el trabajo para desarrollar ese diseño fue parte de la historia que generó la necesidad de hacerlo; es independiente de que necesite hacer refactorización para llegar a ese diseño.

Aprender y comprender el código existente es una cantidad justa de trabajo para un beneficio no permanente (en un mes a partir de ahora es probable que alguien haya olvidado mucho sobre el código si no continúa leyendo o trabajando con él durante ese tiempo), y, por lo tanto, no tiene sentido hacer esto, excepto en las áreas de código que le están causando problemas o que planea cambiar en un futuro cercano. A su vez, dado que este es el trabajo principal de refactorización, no debe hacer refactorización en el código a menos que actualmente le esté causando problemas o planee cambiarlo en un futuro cercano.

Pero hay una excepción a eso: si alguien actualmente tiene una buena comprensión del código que se filtrará con el tiempo, usar esa comprensión para hacer que el código sea más claro y se comprenda más rápidamente más adelante puede ser una buena inversión. Esa es la situación en la que se encuentra alguien que acaba de desarrollar una historia.

  

El refactor es una pequeña pieza de trabajo, y se haría en el siguiente sprint (o incluso antes de que comience) como una pequeña historia de medio punto.

En este caso, si estás pensando en hacer una historia separada para refactorizar es una señal de advertencia en varios frentes:

1. No estás pensando en la refactorización como parte de la codificación, sino como una operación separada, lo que a su vez hace que sea más probable que se caiga bajo presión.

2. Estás desarrollando un código que será más laborioso para entender la próxima vez que alguien necesite trabajar con él, haciendo que las historias tomen más tiempo.

3. Puede estar perdiendo tiempo y energía refactorizando cosas de las que no obtiene muchos beneficios. (Si un cambio ocurre mucho más tarde, alguien tendrá que volver a entender el código, en cualquier caso; eso se combina de manera más eficiente con el trabajo de refactorización. Si un cambio no se produce más adelante, la refactorización no sirvió para nada). propósito en absoluto, excepto quizás uno estético.)

Entonces, la respuesta aquí es fallar el elemento para dejar en claro que algo en su proceso falló (en este caso, es el desarrollador o el equipo que no asigna tiempo para la revisión e implementación de los cambios que salen de revisión) y que el desarrollador continuar de inmediato el trabajo en el artículo.

Cuando vaya a estimar para la próxima iteración, vuelva a estimar la historia existente , ya que parece que queda la cantidad de trabajo para hacer que se apruebe y agregue a su próxima iteración, pero conservando La estimación de la iteración anterior. Cuando se complete la historia al final de la siguiente iteración, establezca la cantidad total histórica de trabajo a la suma de la primera y la segunda estimación, de manera que sepa cuánto trabajo estimado realmente se incluyó. Esto ayudará a producir estimaciones más precisas de historias similares en el futuro en el estado actual de su proceso. (Es decir, no asuma que su subestimación aparente no volverá a suceder; suponga que volverá a suceder hasta que haya completado con éxito historias similares y haya puesto menos trabajo).

en la revisión descubrimos una función que funciona, es legible, pero es bastante larga y tiene algunos olores de código ...

¿Hay algún problema o consideración inherente con la obtención de un ticket de la parte posterior de una revisión, en lugar de fallarlo?

No hay problema en absoluto (en opinión de mis equipos). Supongo que el código cumple con los criterios de aceptación establecidos en el ticket (es decir, funciona). Cree un elemento de reserva para abordar la longitud, y cualquier código huele, y priorícelo como cualquier otro boleto. Si realmente es pequeño, entonces priorícelo alto para el próximo sprint.

Uno de los dichos que tenemos es "Elija una mejora progresiva sobre la perfección pospuesta".

Tenemos un proceso muy fluido, y creamos un número bastante bueno de características de "prueba de concepto" (1 o 2 por sprint) que lo hacen a través del desarrollo y la prueba, pero nunca superan la revisión interna de los interesados (hmm, ¿podemos haz esto en su lugar?), alfa o beta ... algunos sobreviven, otros no.

En el proyecto actual, he perdido la cuenta de cuántas veces hemos creado una característica determinada, la he puesto en manos de los interesados, y un sprint o dos más tarde, la eliminaron por completo porque la dirección del producto ha cambiado, o los requisitos provocaron una refundición completa de cómo debería implementarse la característica. Cualquier tarea de "refinamiento" restante para una función eliminada, o que no se ajuste a los nuevos requisitos, se eliminará, así como parte de la preparación del trabajo acumulado.

En mi opinión, hay dos formas de ver este problema:

1. El camino académico
2. El camino del mundo real

Hablando académicamente, la mayoría de los procesos de revisión de código existen para fallar en la implementación de un PBI (elemento de la cartera de productos) cuando no se cumple el estándar de calidad del código.

Sin embargo, nadie en el mundo real sigue ágil a la T como por una (de muchas razones), diferentes industrias tienen diferentes requisitos. Por lo tanto, corregir el código ahora o asumir deuda técnica (probablemente creará un nuevo PBI) debería decidirse por un por caso Si va a comprometer el sprint o un lanzamiento o introducir una cantidad irrazonable de riesgo, las partes interesadas del negocio deberían participar en la decisión.

Me sorprende la falta de respuesta en las respuestas y comentarios a la noción de "fallar" en una revisión de código, porque ese no es un concepto con el que yo, personalmente, estoy familiarizado. Tampoco me sentiría cómodo con ese concepto ni con nadie de mi equipo que utilice esa terminología.

Su pregunta recurre explícitamente a "prácticas ágiles", por lo que revisemos el manifiesto ágil (el énfasis es mío):

  

Estamos descubriendo mejores formas de desarrollo   Software haciéndolo y ayudando a otros a hacerlo.   A través de este trabajo hemos llegado a valorar:

     

• Personas e interacciones sobre procesos y herramientas
  
• Software en funcionamiento sobre documentación completa
  
• Colaboración con el cliente sobre la negociación del contrato
  
• Respondiendo al cambio sobre seguir un plan
  

Es decir, si bien hay valor en los elementos de   a la derecha, valoramos más los ítems de la izquierda.

Habla con tu equipo. Discuta el código en cuestión. Evalúe los costos y los beneficios y decida, como un grupo cohesionado de expertos, si refactorizar este código ahora, más tarde o nunca.

Comienza a colaborar. Deja de fallar las revisiones de código.

Ninguno . Si falla la revisión del código, la tarea no se realiza. Pero no puedes fallar las revisiones de código en la opinión personal. El código pasa; pasar a la siguiente tarea.

Debería ser una llamada fácil, y el hecho de que no sea así sugiere que no tienes reglas claras y escritas para las revisiones de código.

1. "La función es bastante larga". Anote: las funciones deben tener menos de X líneas (no no sugiriendo que las reglas sobre la duración de la función son una buena cosa).

2. "Hay algunos olores de código". Anote: las funciones públicas deben tener pruebas unitarias de funcionalidad y rendimiento, tanto el uso de la CPU como de la memoria deben estar dentro de los límites x e y.

Si no puede cuantificar las reglas para aprobar una revisión de código, obtendrá este caso de lo que básicamente es 'código que no le gusta'.

¿Deberías fallar el 'código que no te gusta'? Yo diría que no. Naturalmente, comenzarás a aprobar / fallar en función de aspectos que no sean de código: ¿Te gusta la persona? ¿Argumentan fuertemente por su caso o simplemente hacen lo que se les dice? ¿Pasan su código cuando lo revisan?

Además, agrega un paso no cuantificable al proceso de estimación. Calculo una tarea según cómo creo que debería programarse, pero justo al final tengo que cambiar el estilo de codificación.

¿Cuánto tiempo va a agregar eso? ¿El mismo revisor hará la revisión del código posterior y estará de acuerdo con el primer revisor o encontrará cambios adicionales? ¿Qué sucede si no estoy de acuerdo con el cambio y lo pospongo mientras busco una segunda opinión o discuto el caso?

Si desea que las tareas se realicen rápidamente, debe hacerlas lo más específicas posible. Agregar una puerta de calidad vaga no ayudará a su productividad.

Re: ¡¡Es imposible escribir las reglas !!

No es realmente tan difícil. Realmente quieres decir "No puedo expresar lo que quiero decir con 'buen' código" . Una vez que reconoces eso, puedes ver que obviamente es un problema de recursos humanos si empiezas a decir que el trabajo de alguien no está a la altura, pero no puedes decir por qué.

Escriba las reglas que pueda y discuta sobre qué hace que el código sea "bueno".