¿Cuáles son los beneficios / inconvenientes de clasificar los defectos durante una revisión de código par?

8

Hace aproximadamente 3 meses, nuestro grupo de ingeniería lanzó la Junta de Revisión para ser utilizada en todas las revisiones de código de pares. Hoy, tuve una discusión con una de las personas involucradas en ese proceso y descubrí que ya estamos buscando un reemplazo (posiblemente algo comercial) debido a varias características que faltan.

Una de las características que aparentemente solicitan muchas personas es la capacidad de clasificar / categorizar cada comentario de revisión de código (es decir, es un problema de estilo, convención de codificación, pérdida de recursos, error lógico, bloqueo ... lo que sea).

Para aquellos equipos que practican regularmente la revisión de código, ¿es esta categorización una práctica común? ¿Tú lo haces? ¿Lo has hecho en el pasado? ¿Es bueno / malo?

Por un lado, le da al equipo algunas métricas más y posiblemente indicará áreas más específicas en las que los desarrolladores podrían necesitar capacitación (al menos ese parece ser el argumento). ¿Hay otros beneficios? Y, por otro lado, y esta es mi preocupación, es que ralentizará el proceso de revisión de código mucho más. Como líder del equipo, he realizado una gran cantidad de críticas, y siempre me ha gustado la capacidad de resaltar un fragmento de código, elaborar un comentario y avanzar lo más rápido posible. Aunque no lo he probado personalmente, tengo la sensación de que expandir ese cuadro combinado cada vez y desplazarme / buscar la categoría correcta parecería que algo te está tropezando. Además, si comenzamos a mantener métricas sobre estas cosas,

DXM
fuente

Respuestas:

4

Creo que ha respondido en gran medida su propia pregunta sobre los beneficios e inconvenientes relativos.

Las revisiones de código a menudo pueden terminar siendo bastante laboriosas y complicadas, y cualquier cosa que se agregue al tiempo necesario para revisar el código hará que parezca peor. Desde mi punto de vista, desea mantener su proceso de revisión muy corto y no trabajar demasiado en los puntos más delicados. Entonces, la clave es decidir de qué se trata el proceso de revisión que ofrece valor comercial a su equipo.

Las cuestiones de estilo son probablemente uno de los elementos que colocaría como las prioridades más bajas. Claro, mantener el código ordenado y uniformemente formateado puede hacer que sea más fácil de entender, pero preocuparse por el estilo también puede dar lugar a ENORMES ineficiencias durante el proceso de codificación, porque preocuparse por lo bonito que es el código aleja a los desarrolladores de los problemas a resolver. Si todavía le preocupan los problemas de estilo, utilizar una herramienta de verificación de Estilo / Formato (por ejemplo: StyleCop para C #) es una excelente manera de dejar los problemas específicos del estilo hasta el último momento y tomar el proceso de toma de decisiones relacionado con el estilo fuera de las manos de los desarrolladores, liberando su pensamiento para las cosas más importantes. Si no tiene un producto de ese tipo para su idioma de elección, tal vez se pueda escribir un analizador simple para escanear rápidamente su código en busca de tales problemas,

Las fugas de memoria y otros problemas específicos de rendimiento nunca deberían estar a la altura del proceso de revisión. Claro, si ve algo que obviamente causará un problema importante, debe señalarlo, pero no debería ser el propósito de la revisión del código rastrear cada pequeño problema de memoria / rendimiento en su código. Para eso es una buena herramienta de creación de perfiles, y sin duda valen la pena cada céntimo gastado si logras encontrar una muy buena para el lenguaje en el que estás desarrollando.

Los problemas de lógica son problemáticos en el mejor de los casos, y estas son las cosas que realmente pueden absorber mucho tiempo valioso cuando revisa el código. En lugar de dejar todo esto completamente para la revisión del código, esto es para lo que deberían usarse sus pruebas unitarias. Sí, incluso las pruebas pueden ser incorrectas; sin embargo, si desarrolla la prueba primero, respete los principios SRP y DRY, refactorice sin piedad y defina las pruebas unitarias como un medio para validar sus especificaciones, encontrará que terminará con muchos menos problemas relacionados con la lógica. Si realiza la prueba después de codificar, es menos probable que se enfrente a posibles problemas lógicos a medida que surgen, y es más probable que se olvide de probar una ruta particular a través de su código.

Entonces, si haces todo como te he sugerido aquí, ¿qué te deja hacer en la revisión del código? La respuesta simple es que su revisión de código se convierte en un proceso bastante simple por el cual el codificador explica al revisor cómo se ha capturado un requisito particular en las pruebas y cómo se han aplicado esas pruebas a los problemas resueltos. Tiende a verificar más el código y analiza las pruebas más a fondo, ya que aquí es donde se puede medir el mayor valor comercial, y particularmente cuando ese código debe mantenerse más adelante. Para que la revisión del código de prueba sea aún más indolora, el uso de un buen marco de prueba conducido por el comportamiento puede simplificar enormemente la revisión, ya que las especificaciones se capturan en el código como una descripción en inglés casi simple sobre cómo se ejecutará la prueba. Se realizan verificaciones detalladas de cualquier código que respalde las pruebas, y si su marco BDD produce buenos informes de texto que enumeran las pruebas en declaraciones de historia / función de texto plano, entonces el proceso se vuelve aún más fácil. Todo esto se suma a un proceso extremadamente eficiente que será tan valioso como una revisión de código más tradicional, pero que se puede llevar a cabo mucho más rápido y de una manera más enfocada, lo que ayuda a evitar atascarse en trivialidades y verificaciones dobles que a menudo no te llevan a ninguna parte. Este enfoque más ágil significa más tiempo dedicado a las pruebas y la codificación, y menos tiempo en procesos administrativos. pero puede llevarse a cabo mucho más rápido y de una manera más enfocada, ayudándole a evitar atascarse en trivialidades y verificaciones dobles que a menudo no lo llevan a ninguna parte. Este enfoque más ágil significa más tiempo dedicado a las pruebas y la codificación, y menos tiempo en procesos administrativos. pero puede llevarse a cabo mucho más rápido y de una manera más enfocada, ayudándole a evitar atascarse en trivialidades y verificaciones dobles que a menudo no lo llevan a ninguna parte. Este enfoque más ágil significa más tiempo dedicado a las pruebas y la codificación, y menos tiempo en procesos administrativos.

Entonces, ¿qué pasa con esas métricas entonces? Si todos los problemas se tratan simplemente como "errores", entonces la única medida con la que realmente debe preocuparse es la cantidad de errores que encuentra antes y después de la liberación, a lo largo del tiempo, y si los errores identificados tienen tendencia en alguna dirección en particular. Si está utilizando incluso un sistema de seguimiento de problemas medio decente, toda esta información estará prácticamente a su alcance, y no tendrá que preocuparse por si su proceso de revisión necesita identificarlos. Al final del día, su equipo quiere hacer lo que son buenos, que es escribir software, y no dedicar demasiado tiempo a cuestiones administrativas que a menudo solo ofrecen algo de interés para 1 o 2 personas en la empresa.

S.Robins
fuente
1

... por otro lado, y esta es mi preocupación, es que ralentizará el proceso de revisión de código ...

Si desea mantener las revisiones livianas, entonces las tareas que requieren mucho esfuerzo, como la investigación, las reparaciones y las pruebas de defectos descubiertos, no deberían estar allí.

  • La opción directa ( tonta si lo desea) para manejar eso es, bueno, simplemente suéltela y espere hasta que los evaluadores encuentren e informen el error ellos mismos. El principal inconveniente de este enfoque es que algunos problemas que son fácilmente visibles en el código fuente pueden requerir muchos esfuerzos para descubrir en las pruebas de caja negra.
     
    Los problemas de subprocesos y las pérdidas de memoria a menudo entran en esta categoría. La carrera de datos que me toma un minuto detectar mediante la observación del código, puede permanecer allí durante meses, pasando todas las pruebas antes de que levante su cabeza fea.

Debido a lo anterior, nunca uso y no recomiendo una opción como la anterior.

  • Otra opción es retirar el trabajo pesado, en el sentido de que mi revisor trabaja en la cola de trabajo en otro lugar. Esta opción es la que uso, con la que me siento bastante cómodo.
     
    El enfoque es bastante simple. Si siento que los problemas que encontré pueden tomar un esfuerzo considerable para tratar, simplemente creo un ticket en el rastreador de problemas con un título como "investigar y abordar los problemas señalados en la revisión de código <ID / URL de revisión>" y seguir adelante a través del código haciendo comentarios de forma gratuita. No pierdo tiempo en cosas que se harán por boleto que abrí, porque entrará en la cola de trabajo en equipo y se priorizará y se tratará de manera rutinaria.
     
    De esa manera, me aseguro de que 1) el proceso de revisión sea lo más liviano posible y 2) los problemas descubiertos no serán olvidados.
mosquito
fuente
0

Parece que estás buscando codetriker . Tiene una función para clasificar problemas tanto por modo, nivel y tipo.

Para aquellos equipos que practican regularmente la revisión de código, ¿es esta categorización una práctica común?

Bueno, ya que uso la herramienta anterior, me veo obligado a hacerlo. El problema que veo con eso es que a veces puede ser difícil hacerlo bien, porque hay muchas opciones.


La clasificación de los problemas proporciona poco valor a la revisión del código, ya que el comentario es más importante. En realidad, dado que me veo obligado a establecer 4 campos en la herramienta anterior para cada comentario, se volvió engorroso.

Más adelante, cuando verifique el resumen de las revisiones, podrá ver que la distribución parece tener una distribución única. Las personas tienden a no cometer el mismo error varias veces.

BЈовић
fuente
gracias pero no estaba buscando una herramienta que pueda hacerlo. Estoy buscando personas que utilizaron la función de clasificación en sus revisiones y quiero escuchar su opinión sobre dicha función. ¿Es útil? ¿Agrega valor para los desarrolladores y / o la administración? ¿O intentar clasificar las cosas simplemente desperdicia tiempo y desvía la atención de lo que es realmente importante?
DXM
@DXM Ok, en ese caso no entendí bien la pregunta. Con suerte lo respondí en la edición
BЈовић
0

Codificamos la revisión usando una simple Causa Mayor / Menor y Raíz (Requisitos / Diseño / Implementación). Mantener bajos los elementos numéricos proporciona métricas utilizables, sin la carga de ser demasiado fino. Raramente hay una discusión sobre una categoría, ya que hay muy pocos para elegir.

Nuestro seguimiento de revisiones es antiguo, pero ha evolucionado a lo largo de 20 años. La herramienta de "elección" es Excel, por lo que ya es onerosa. Estamos cambiando a Gerrit para revisiones de código menos formales (fuera de "El proceso", ya que se ajusta mejor a las necesidades del desarrollador, con gerrit simplemente intercalamos en el comentario y seguimos adelante. No captura de "métricas".

Personalmente, creo que las métricas están sobrevaloradas: la revisión trata sobre la mejora del código, y le pregunto a cualquiera que se haya sentado sobre algunas, que le dirán lo mismo que las métricas. Sin embargo, las métricas no se pueden discutir. También se pueden manipular de manera útil según sea necesario, y la administración no es lo más sabia (a veces soy un poco cínico).

Mattnz
fuente
Gerrit? No he oído hablar de él antes (aparte del hecho de que es un nombre holandés para chicos). ¿Podría agregar un enlace para que pueda encontrar más información?
Marjan Venema