Prácticas ágiles: revisión de código: ¿no pasa la revisión o plantea un problema?

53

Al final de un sprint de 2 semanas y una tarea tiene una revisión de código, en la revisión descubrimos una función que funciona, es legible, pero es bastante larga y tiene algunos olores de código. Fácil trabajo de refactorización.

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

Tenemos dos opciones.

  • Si no se revisa el código, el boleto no se cerrará en este sprint y daremos un pequeño golpe de moral, porque no podemos pasar el boleto.
  • El refactor es un pequeño trabajo, y se haría en el próximo sprint (o incluso antes de que comience) como una pequeña historia de medio punto.

Mi pregunta es: ¿hay algún problema o consideración inherente al levantar un ticket de la parte posterior de una revisión, en lugar de fallar?

Los recursos que puedo encontrar y he leído reseñas detalladas de códigos como 100% o nada, por lo general, pero creo que eso generalmente no es realista.

Erdrik Ironrose
fuente
23
Entonces, si no puede fallar la revisión del código para eso, ¿cuál es el propósito de la revisión? En este momento, parece que es para ver si algo funciona , sin embargo, seguramente ese es el trabajo de una prueba o un probador, no una revisión de código.
VLAZ
21
Creo que la mayoría de las respuestas pierden un punto importante en su pregunta: de lo contrario, la tarea se ajusta a la definición de hecho. ¿Los problemas que menciona son parte de lo que su equipo considera una tarea "no realizada"? ¿O estos problemas no se consideran en lo que debería ser una tarea "hecha"? Si su definición de "hecho" incluye "sin olores de código", entonces la tarea simplemente no está hecha.
Josh Part
1
@ErdrikIronrose, por lo que parece que el cambio no estuvo a la altura y posiblemente no fue (tan fácilmente) mantenible. Aunque su otro comentario parece indicar que el cambio no fue parte de la solicitud, en cuyo caso no debería ser parte de la revisión del código. Si alguien escribe un código correcto y estándar al lado de un truco feo existente, entonces, siéntase libre de hacerlo, levante una multa por arreglar el truco feo y pase la revisión del código actual. Si alguien escribe un código que es correcto pero no está a la altura (como lo indica su pregunta), no complete la revisión del código hasta que se haya realizado correctamente.
VLAZ
99
@ErdrikIronrose: Ah, ¿entonces el código olfativo no se creó mientras se trabajaba en la historia que se estaba revisando , pero ya existía? Esa es una distinción importante: considere editar la pregunta.
sleske
1
@vlaz deberías responder a tu comentario
Ister

Respuestas:

67

¿Hay algún problema o consideración inherente al levantar un ticket de la parte posterior de una revisión, en lugar de fallar?

No inherentemente Por ejemplo, la implementación del cambio actual puede haber descubierto un problema que ya estaba allí, pero que hasta ahora no era conocido / aparente. Fallar el boleto sería injusto ya que lo fallarías 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 fue agregado por el cambio actual. En este caso, el boleto debe fallar ya que el código no pasó la prueba de olor.

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

Si no se revisa el código, el boleto no se cerrará en este sprint y daremos un pequeño golpe de moral, porque no podemos pasar el boleto.

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

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

El refactor es un pequeño trabajo, y se haría en el próximo 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 boleto si el olor del código no se puede atribuir directamente al boleto original (por ejemplo, un escenario de "gota que colmó el vaso").

Los recursos que puedo encontrar y he leído reseñas detalladas de códigos como 100% o nada, por lo general, pero creo que eso generalmente no es realista.

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

Creo que a lo que te estás refiriendo aquí es más que interpretas que las revisiones de código requieren un código perfecto o de lo contrario fallan, y ese no es el caso.

El código no debe ser impecable, simplemente debe cumplir con el estándar razonable de limpieza que emplea su equipo / empresa. La adhesión a ese estándar es una elecció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 debe aprobarse 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" fuera 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 unitarias ya serían nuestro proceso de revisión automatizado y no necesitaría revisiones de código o argumentos de estilo.

Flater
fuente
26
"La exactitud y limpieza del código no depende del estado de ánimo del equipo". +1 solo por esto, sin embargo, la única advertencia a toda esta respuesta sería llegar a una fecha límite. Si fallar esta revisión del código significa que una característica muy esperada no llegará a la próxima versión, debe equilibrar la limpieza del código con las necesidades del cliente. Pero recuerde que el código incorrecto que cumple con la fecha límite del cliente hoy es un problema de producción mañana.
Greg Burghardt
11
Gran respuesta: firme pero no grosero. Un punto tangencial también puede ser: ¿cómo llegamos a hacer revisiones de código tan tarde en el sprint que no se pudo hacer un refactorizador fácil sin que fallara todo el sprint?
Daniel
@Daniel: el desarrollador puede estar comprometido o puede ser un problema de planificación. El tiempo entre terminar una tarea y terminar el sprint es generalmente mínimo ya que (en un mundo ideal) las personas terminarían su última tarea del sprint alrededor del tiempo de cierre del sprint. No puede tomar un período prolongado para revisar / corregir; o, alternativamente, tal vez el desarrollador simplemente no esté presente / disponible para el resto del sprint.
Flater
8
Los programadores de +1 pueden sentirse bien cuando han escrito un buen código. Eludir su control de calidad no es la respuesta para mejorar la moral. De todos modos, no es probable que un rechazo ocasional por problemas menores afecte la moral. Si su moral está sufriendo debido a la falta regular de pasar el control de calidad, la respuesta es hacer algo sobre fallar el control de calidad todo el tiempo, no abandonar los estándares.
jpmc26
1
@GregBurghardt: Debido a que he visto que se abusa del argumento de la fecha límite en muchas empresas, tiendo a aceptar pasar una mala revisión solo si y solo si se crea y se planifica una tarea para su refactorización inmediata para el primer sprint posterior al lanzamiento. El costo de tiempo adicional agrega una barrera de entrada significativa para eludir la calidad del código.
Flater
38

Al final de un sprint de 2 semanas y una tarea tiene una revisión de código [...] Fácil trabajo de refactorización.

¿Por qué aparece eso al final del sprint? Una revisión del código debe ocurrir tan pronto como piense que el código está hecho (o incluso antes). Debe verificar su definición de hecho con cada historia que termine.

Si se encuentra terminando historias tan poco antes de su revisión de demostración / sprint que no puede encajar en una tarea "pequeña", entonces necesita mejorar para estimar su trabajo. Sí, esa historia no se terminó. No por una revisión de código, sino porque no planeaba incorporar cambios de la revisión de código. Eso es como estimar que las "pruebas" toman tiempo cero, porque "si lo programó correctamente, simplemente funcionará, ¿verdad?". Así no es como funciona. Las pruebas encontrarán errores y la revisión del código encontrará cosas que cambiar. Si no fuera así, 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 planifica adecuadamente, no llegará a esa etapa de "hecho" a tiempo y quedará atrapado en el territorio de "no hecho" al final del sprint. Eso no es bueno para la moral, pero debes tenerlo en cuenta en la planificación.

nvoigt
fuente
55
Esta es exactamente la respuesta que me vino a la mente. Si cada historia se implementa con su propia rama, no posponga la revisión y fusión de las ramas hasta el final del sprint. En su lugar, cree una solicitud de extracción tan pronto como se crea que la rama está lista, y siga iterando en esa rama hasta que esté realmente hecha, aprobada y fusionada. Si ese proceso no ha terminado al final del sprint, entonces la historia no ha terminado.
Daniel Pryden
20

Simple: revisas el cambio . No revisa el estado del programa de otra manera. Si soluciono un error en una función de 3.000 líneas, verifica que mis cambios solucionen el error, y eso es todo. Y si mi cambio corrige el error, usted acepta el cambio.

Si crees que la función es demasiado larga, entonces pones una solicitud de cambio para acortar la función, o la divides, después de que mi cambio fue aceptado, y esa solicitud de cambio se puede priorizar según su importancia. Si se toma la decisión de que el equipo tiene cosas más importantes que hacer, entonces se maneja más tarde.

Sería ridículo si pudieras 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 elevar un ticket basado en lo que vio al revisar el cambio. En algunos casos, incluso hará esto si el cambio en sí mismo causó los problemas: si es más importante tener los cambios ahora que solucionar los problemas. Por ejemplo, si otros fueron bloqueados, esperando el cambio, entonces desea desbloquearlos mientras se puede mejorar el código.

gnasher729
fuente
44
Creo que en este caso el cambio fue la función demasiado larga, si ha introducido una función de 3000 líneas que no estaba allí anteriormente (o era una función de 10 líneas anteriormente).
user3067860
3
En principio, esta respuesta es exactamente correcta. En la práctica ..... Si todos los desarrolladores creen y practican buenas prácticas de codificación equilibradas contra el esfuerzo, entonces es probable que no te encuentres con este problema muy a menudo y luego esta respuesta sea acertada. Sin embargo ... parece que siempre hay uno o dos desarrolladores que hacen todo rápido y sucio para ahorrar 5 minutos ahora; mientras que ignoran las horas a días o meses que están agregando al trabajo que será más tarde. En esos casos, esta respuesta es solo una pendiente resbaladiza para tener que comenzar de nuevo y rediseñar todo el sistema.
Dunk
+1, aunque creo que debería reformular el último párrafo para que se destaque que registrar el código con problemas debería ser una excepción absoluta. Quiero decir, solo que alguien esté bloqueado no es suficiente excusa. Fallar un solo sprint tampoco parece una excusa suficiente, y ciertamente no es una excusa que pueda usarse repetidamente.
Frax
@ user3067860 Si ha convertido una función de 10 líneas en una función de 3000 líneas, entonces claramente falla. Si ha convertido una función de línea 3000 en 3010, entonces probablemente pase. Pero, ¿qué pasa si ha convertido una función de 100 líneas (generalmente un poco demasiado grande) en una función de 300 líneas ( definitivamente demasiado grande)?
Martin Bonner apoya a Monica el
9

Si no se revisa el código, el boleto no se cerrará en este sprint y daremos un pequeño golpe de moral, porque no podemos pasar el boleto.

Este parece ser el problema.
En teoría, sabes lo que debes hacer, pero está cerca de la fecha límite, por lo que no quieres hacer lo que sabes que debes hacer.

La respuesta es simple: haga lo que haga si obtiene el mismo código para la revisión del código el primer día del sprint. Si fuera aceptable, debería serlo ahora. Si no fuera así, no lo sería ahora.

xyious
fuente
"Estimado cliente, no puede tener su función durante otras 2-3 semanas porque nuestro código funcionó, pero no nos gustó cómo se veía", ... por favor, no vaya a nuestro competidor ... ni se lo diga al CEO !
RandomUs1r
66
Los clientes de @ RandomUs1r no deberían tener ese tipo de información. No se hizo porque no había suficiente tiempo y eso es todo. ¿Los clientes dictan cómo se debe escribir el código? Si llama a un electricista para que arregle el cableado de su hogar, ¿dice "Simplemente cambie los cables pero no se moleste en verificar si son los cables correctos"? ¿O le dice a su médico "Estoy enfermo, deme algunas pastillas pero no me diagnostique primero"? Las revisiones de código deben ser una parte inherente del trabajo, no algo que el cliente dicta.
VLAZ
1
@ RandomUs1r: "" Estimado desarrollador, ¿por qué no se completó la función? " , La respuesta debería ser" porque no tuvimos el tiempo suficiente para construirla a un nivel de calidad aceptable ", tal vez seguido de" Podemos darle para ti si estás dispuesto a comprometer la calidad ".
Bryan Oakley
1
@ RandomUs1r, por lo que básicamente desea sacrificar la calidad del código, ahora es probable que sea mucho más difícil implementar funciones más adelante. Una solución de 2 días ahora podría ahorrarle una solución de 4 semanas más tarde. Entonces, es "Estimado cliente, no puede tener su función por otras 2-3 semanas porque lleva mucho tiempo implementar una función menor ahora". ¿También es el final de un sprint o es una fecha límite importante? Si es una fecha límite importante, podría ver la fusión ahora, escribir una solución durante los próximos 2 días y aumentar un PR justo después de la fecha límite.
xyious
55
Todo lo que digo es que si sus estándares son diferentes el primer día y el último día del sprint, entonces no tiene un estándar y su calidad inevitablemente se deteriorará.
xyious
5

Una gran parte del proceso es decidir qué significa hacer y apegarse a sus armas. También significa no comprometerse demasiado y realizar sus revisiones por pares 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 en que puede 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.
  • Puede devolverlo con comentarios sobre lo que está mal. Si el manejo de errores se realiza mal, o el desarrollador sigue repitiendo los mismos errores, esto puede justificarse.
  • Puede crear un boleto e incurrir en deudas técnicas. El boleto está ahí para asegurarse de pagarlo más tarde. Es posible que tenga poco tiempo y que en el proceso de revisar los cambios vea 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 mayores que los que trabajó el desarrollador, levante la bandera y siga adelante. Pero no deberías estar en una posición en la que haya horas antes del final del sprint y justo ahora estás recibiendo una revisión por pares. Eso huele a comprometer excesivamente sus recursos, o posponer las revisiones por pares. (un olor a proceso).

Berin Loritsch
fuente
4

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

  1. ¿Cuál es el propósito de su revisión de 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 compuerta, ¿qué problemas se consideran 'bloqueadores'?

Todo esto se reduce a lo que el equipo ha acordado 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 cumpla con este requisito.

Es lo mismo que si durante la prueba unitaria fallara una prueba unitaria. Solucionaría el error, no ignoraría la prueba unitaria, si pasar las pruebas unitarias era un requisito para realizar.

Si el equipo no ha aceptado que las revisiones de código sean una definición de Listo, entonces sus revisiones de código no son una prueba de aceptación indirecta 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 elementos de trabajo nuevos para que el equipo los priorice.

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

Jay S
fuente
1

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

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

  1. Si solo hace que el código sea más claro y conciso, los cambios necesarios son obvios. A menudo entendiste el código al probar cambios que parecían más limpios y ver si realmente funcionaban, o si se perdían 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 característica. En ese caso, el trabajo para desarrollar ese diseño fue parte de la historia que generó la necesidad del mismo; es independiente de que necesite refactorizar para llegar a ese diseño.

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

Pero hay una excepción a eso: si alguien 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 entienda 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 terminar de desarrollar una historia.

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

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

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

  2. Estás desarrollando un código que será más difícil de entender la próxima vez que alguien necesite trabajar con él, lo que hará que las historias tarden más.

  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 comprender el código, de todos modos; eso se combina de manera más eficiente con el trabajo de refactorización. Si un cambio no ocurre 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, ese es el desarrollador o el equipo que no asigna tiempo para la revisión e implementa los cambios que salen de la revisión) y que el desarrollador continúe trabajando inmediatamente en el artículo

Cuando vaya a estimar para la próxima iteración, vuelva a estimar la historia existente como cualquier cantidad de trabajo que parezca quedar para hacer que pase la revisión y agregarla a su próxima iteración, pero conservando la estimación de la iteración anterior. Cuando la historia se complete al final de la siguiente iteración, establezca la cantidad total histórica de trabajo en la suma de las estimaciones primera y segunda para que sepa cuánto trabajo estimado realmente se puso en ella. 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 aparente subestimación no volverá a suceder; suponga que volverá a suceder hasta que haya completado con éxito historias similares mientras trabaja menos).

Curt J. Sampson
fuente
1

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

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

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

  • Individuos e interacciones sobre procesos y herramientas.
  • Software de trabajo sobre documentación completa
  • Colaboración del cliente sobre negociación de contrato
  • Responder al cambio sobre seguir un plan

Es decir, si bien hay valor en los elementos de la derecha, valoramos más los elementos 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 coherente de expertos, si refactorizará este código ahora, más tarde o nunca.

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

Hormiga P
fuente
Estoy a favor de la colaboración. Pero, ¿qué término usarías, si no "falla"? Incluso discutiendo, como grupo, una persona diría "esto no es lo suficientemente bueno, necesita una refactorización", lo que significa, simplemente, que falló el control de calidad, ¿verdad?
Erdrik Ironrose
1
@ErdrikIronrose Nunca he usado, o necesitaba usar, la terminología de "fallar" una revisión de código. Alguien revisa el código, se produce una discusión sobre cualquier punto potencial de mejora, seguido de una decisión sobre si abordar esos puntos. No hay "pases" o "fallas" involucrados, solo comunicación y progreso. No estoy seguro de por qué es necesario un sello de goma.
Ant P
0

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 al levantar un ticket de la parte posterior de una revisión, en lugar de fallar?

No hay problema alguno (en la opinión de mi equipo). Supongo que el código cumple con los criterios de aceptación establecidos en el ticket (es decir, funciona). Cree un elemento de la cartera de pedidos para abordar la longitud y cualquier olor a código, y priorícelo como cualquier otro ticket. Si realmente es pequeño, simplemente 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 una cantidad bastante buena de características de 'prueba de concepto' (1 o 2 por sprint) que pasan por el desarrollo y la prueba, pero nunca pasan la revisión interna de las partes interesadas (hmm, ¿podemos hacer esto en su lugar? ?), alfa o beta ... algunos sobreviven, otros no.

En el proyecto actual, he perdido la cuenta de cuántas veces hemos construido una determinada característica, la he puesto en manos de las partes interesadas y un sprint o dos más tarde, la eliminé por completo porque la dirección del producto ha cambiado o los requisitos han causado Un repaso completo de cómo se debe implementar la función. Cualquier tarea de 'refinamiento' restante para una característica eliminada, o que no cumpla con los nuevos requisitos, se eliminará, así como parte de la preparación del trabajo atrasado.

railsdog
fuente
0

Hay dos formas de ver este problema en mi opinión:

  1. La forma académica
  2. El 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 ya que por una (de muchas razones), diferentes industrias tienen diferentes requisitos. Por lo tanto, arreglar el código ahora o asumir una deuda técnica (lo más probable es que cree un nuevo PBI) debe decidirse caso por caso. Si va a comprometer el sprint o un lanzamiento o introducir una cantidad de riesgo irrazonable, las partes interesadas del negocio deben participar en la decisión.

RandomUs1r
fuente
2
nadie en el mundo real sigue ágil a la T - ya no será "ágil" si tenemos reglas demasiado estrictas, ¿verdad?
Paŭlo Ebermann
@ PaŭloEbermann Tuve una conversación divertida con una empresa que entrevisté una vez. Afirmaron que su proceso no era ágil porque no era un ejemplo de libro de texto de ágil. A pesar de que todo lo que hicieron fue en el espíritu de ágil. Se lo señalé, pero solo me encontré con (esencialmente) "No, no estamos siguiendo un procedimiento ágil establecido al pie de la letra, incluso si tomamos prestados los conceptos en gran medida. Por lo tanto, no somos ágiles". Fue bastante extraño.
VLAZ
Como han señalado otros revisores, en este caso hay potencialmente una lección que aprender del fracaso del código para aprobar realmente la revisión. Me parece que la gente de este proyecto realmente no comprende que a) es necesario dejar tiempo para revisar y corregir cada historia, yb) la refactorización necesaria para dejar un código limpio es una parte esencial de historia. En ese caso, lo mejor que puede hacer es fallar la historia para dejar en claro que estas cosas realmente no son opcionales.
Curt J. Sampson
@Curt Entiendo que la mía puede ser una vista impopular desde el punto de vista del desarrollador (yo también soy un desarrollador), pero el negocio realmente debería ser lo primero, firman los cheques de pago y eso merece un poco de respeto. En lo que respecta al tiempo libre, volveré a desafiar tu comprensión del mundo real, y debes darte cuenta de que eso no siempre es posible y que muchos sprints funcionan bien porque los desarrolladores también necesitan hacer cosas al final del sprint. No es como porque el código es SÓLIDO, un departamento puede levantar los pies 1/10 días cada 2 semanas y no hacer nada, eso podría ser excelente a corto plazo, pero no es viable a largo plazo.
RandomUs1r
@ RandomUs1r Yo también trabajo en el mundo real, tomo atajos todo el tiempo y siempre pongo el negocio primero, así que no creo que me falte comprensión aquí. Pero la descripción del OP no era "normalmente siempre hacemos esto bien y esto era solo un eructo menor estándar" o no habría estado publicando la pregunta. Como expliqué en mi respuesta , parece un problema de proceso, y lo arreglas practicando el proceso correctamente antes de relajarte.
Curt J. Sampson
-2

Ninguno . Si falla la revisión del código, entonces la tarea no está completa. Pero no puede fallar las revisiones de código en 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 lo sea sugiere que no tiene reglas escritas lo suficientemente claras para las revisiones de código.

  1. "La función es bastante larga". Anote: Las funciones deben ser inferior a las líneas de largo X (estoy no sugiriendo que las normas sobre la longitud de la función son una cosa buena).

  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 por debajo 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ía fallar el "código que no le gusta"? Yo diría que no. Naturalmente, comenzará a aprobar / reprobar en función de aspectos que no sean de código: ¿le gusta la persona? ¿Argumentan fuertemente por su caso o simplemente hacen lo que se les dice? ¿Pasan tu código cuando te revisan?

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

¿Cuánto tiempo agregará eso? ¿Hará el mismo revisor la revisión de código posterior y estará de acuerdo con el primer revisor o encontrarán cambios adicionales? ¿Qué sucede si no estoy de acuerdo con el cambio y pospongo hacerlo mientras busco una segunda opinión o argumento 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 reconoce eso, puede ver que obviamente es un problema de recursos humanos si comienza a decir que el trabajo de alguien no está a la altura, pero no puede decir por qué.

Escriba las reglas que pueda y tenga discusiones sobre la cerveza sobre lo que hace que el código sea 'bueno'.

Ewan
fuente
66
No, se está perdiendo el punto de que "tener un estándar perfecto y universalmente aplicable sin ambigüedades" no es un requisito previo realista para realizar revisiones de código. Siempre habrá nuevos tipos de problemas que aún no había tenido en cuenta y, por lo tanto, debe poder tomar una decisión en un territorio desconocido. Por supuesto, debe documentar esa decisión para que ya no sea un territorio desconocido, pero su respuesta se basa en la suposición de que de alguna manera puede garantizar la ausencia de territorio desconocido si solo redacta las reglas perfectas antes de la revisión. Estás poniendo el carro delante del caballo.
Flater
55
Absolutos como "las funciones deben tener menos de x líneas de largo" tampoco son la respuesta .
Blrfl
2
De acuerdo con Blrfl. Las funciones (en general) no deben tener más de 20 líneas. Pero convertirlo en una regla absoluta es un error. Las circunstancias específicas siempre prevalecen sobre las reglas generales: si tiene una buena razón para hacer que su función supere las 20 líneas, hágalo.
Matt Messersmith
1
No debería necesitar reglas para el código escrito con una especificación legal ... Solo puede tener pautas más el hecho de que presumiblemente son adultos que están tratando de lograr el mismo objetivo final (código funcional, legible y mantenible). Tener a todos los miembros del equipo genuinamente invertidos en el equipo y dispuestos a trabajar juntos es fundamental para Scrum, por lo que si no lo tiene, tal vez Scrum no sea para su equipo de todos modos.
user3067860
2
@Ewan Claro. Mi punto era solo que el OP tiene una longitud de guía de función , no una regla. Dondequiera que se establezca el umbral, proporciona consejos para ayudar a las personas a detectar códigos difíciles de mantener, pero nunca es una regla absoluta. Si (como dice el OP) en realidad es perfectamente legible, y los revisores están de acuerdo en que es perfectamente legible, y no hay problemas para probarlo adecuadamente, entonces la función es, por definición, el tamaño correcto. La revisión tal vez reciba una nota de una línea que dice "Sí, es más largo de lo recomendado, pero estamos de acuerdo en que está bien", y el trabajo está hecho. Refactorizar después de ese punto es chapado en oro.
Graham