Buenas pautas y prácticas para la revisión obligatoria del código [cerrado]

Estamos probando la revisión obligatoria del código en cada confirmación (nada entra en el maestro que no haya sido validado por al menos 1 persona, no el autor) durante un par de sprints. Tenemos la aceptación tanto de los desarrolladores como de la administración (que es una situación increíble) y queremos obtener algunos de los beneficios por los que es conocida:

• reducción obvia de errores
• mayor conciencia de los cambios que ocurren alrededor del proyecto
• el "Sé que alguien va a ver esto, así que no seré flojo" / efecto anti-vaquero
• mayor consistencia dentro / a través de proyectos

Pero estamos introduciendo algo que se sabe que reduce la velocidad, y si se hace mal, podría crear un paso burocrático estúpido en la tubería de compromiso que no hace nada más que tomar tiempo. Cosas que me preocupan:

• las revisiones se convierten en solo selección de liendres
• (hiperbólicamente) personas abriendo grandes problemas arquitectónicos como parte de una revisión de confirmación de dos líneas.
• No quiero sesgar las respuestas con otras cosas.

Si bien todos somos personas razonables y haremos un gran autoanálisis, definitivamente podríamos usar una idea ganadora de la batalla sobre qué tipo de cosas deberíamos estar tratando de lograr en una sesión de revisión para que las revisiones realmente funcionen para nosotros . ¿Cuáles son algunas pautas y políticas que ha encontrado que funcionan?

1. Haga comentarios cortos.

   Es difícil mantenerse concentrado, especialmente durante la revisión del código, durante mucho tiempo. Además, las revisiones largas de código pueden indicar que hay mucho que decir sobre el código (ver los siguientes dos puntos) o que la revisión se convierte en una discusión sobre temas más importantes, como la arquitectura.

   Además, una revisión debe seguir siendo una revisión, no una discusión. No significa que el autor del código no pueda responder, pero no debería convertirse en un largo intercambio de opiniones.

2. Evite revisar el código que es demasiado malo.

   Revisar el código de baja calidad es deprimente tanto para el revisor como para el autor del código. Si un fragmento de código es terrible, una revisión de código no es útil. En cambio, se le debe pedir al autor que reescriba el código correctamente.

3. Use verificadores automáticos antes de la revisión.

   Los verificadores automáticos evitan perder tiempo valioso señalando errores que se pueden encontrar automáticamente. Por ejemplo, para el código C #, ejecutar StyleCop, las métricas de código y especialmente el análisis de código es una buena oportunidad para encontrar algunos de los errores antes de la revisión. Luego, la revisión del código puede gastarse en puntos que son extremadamente difíciles de hacer para una máquina.

4. Elija cuidadosamente a las personas que hacen comentarios.

   Dos personas que no pueden soportar el uno al otro no harán una buena revisión del código de otro. El mismo problema surge cuando una persona no respeta a la otra (no importa si es el revisor o el autor, por cierto).

   Además, algunas personas simplemente no pueden ver su código revisado, por lo que requieren capacitación y preparación específicas para comprender que no son criticadas y que no deberían verlo como algo negativo. Hacer una revisión, sin preparación, no ayudará, ya que siempre estarán a la defensiva y no escucharán ninguna crítica de su código (tomando cada sugerencia como crítica).

5. Haz revisiones tanto informales como formales.

   Tener una lista de verificación ayuda a concentrarse en un conjunto preciso de fallas, evitando que se convierta en la selección de liendres. Esta lista de verificación puede contener puntos como:

   • Inyección SQL,
   • Suposiciones erróneas sobre un lenguaje que puede conducir a errores,
   • Situaciones específicas que pueden conducir a errores, como la precedencia del operador. Por ejemplo, en C #, var a = b ?? 0 + c ?? 0;podría verse bien para alguien que quiera agregar dos números anulables con fusión en cero, pero no lo es.
   • Desasignación de memoria,
   • Carga diferida (con sus dos riesgos: cargar la misma cosa más de una vez y no cargarla en absoluto),
   • Desbordamientos,
   • Estructuras de datos (con errores como una lista simple en lugar de un conjunto hash, por ejemplo),
   • Validación de entrada y programación defensiva en general,
   • Hilo de seguridad,
   • etc.

   Dejo la lista aquí, pero hay cientos de puntos que pueden figurar en una lista de verificación, dependiendo de los puntos débiles de un autor preciso.

6. Ajuste progresivamente la lista de verificación.

   Para mantenerse constructivo y útil a lo largo del tiempo, las listas de verificación utilizadas en las revisiones formales deben ajustarse con el tiempo dependiendo de los errores encontrados. Por ejemplo, las primeras revisiones informales pueden revelar una cierta cantidad de riesgos de inyección SQL. La comprobación de inyección SQL se incluirá en la lista de verificación. Cuando, unos meses después, parece que el autor ahora tiene mucho cuidado con las consultas dinámicas frente a las parametrizadas, la inyección SQL puede eliminarse de la lista de verificación.

Tenemos casi como una lista de verificación:

• Muéstrame la descripción de la tarea.
• Guíame sobre el resultado y muéstralo funcionando. Ejecute diferentes escenarios (entrada no válida, etc.).
• Muéstrame las pruebas de aprobación. ¿Cómo es la cobertura de prueba?
• Muéstrame el código: ahí es donde buscamos ineficiencias obvias.

Funciona bastante bien

Creo que una persona que tiene poder sobre los demás sería suficiente, administrador o moderador para cortar comentarios irrelevantes, acelerar la revisión de cosas que necesitan una revisión rápida. Único tomador de decisiones.

Menos de esto sería que esta persona tiene que hacerlo como tarea principal, mientras que él podría estar haciendo otra cosa, y probablemente le gustaría tener a la persona más experimentada en esta posición.

¡Lo segundo es automatizar tanto como puedas!

• controlando espacios en blanco
• software de control de estilo
• compilaciones automatizadas antes de la revisión del código
• pruebas automatizadas antes de la revisión del código

Esas cosas eliminarán al menos algunas cosas que las personas podrían comentar sin necesidad real. Si no está construyendo o tiene espacios en blanco al final, no es lo suficientemente bueno para la revisión, corríjalo y solicite la revisión nuevamente. Si no se está construyendo o alguna prueba falla, entonces es obvio que no es lo suficientemente bueno.

Mucho depende de sus tecnologías, pero encuentre lo que puede verificar automáticamente cuanto más mejor.

Todavía no hemos ganado esta batalla, pero eso es lo que encontramos útil.