¿Cómo monitorear la revisión de código de manera eficiente?

28

Sospecho que en mi equipo se encubrieron importantes revisiones de código. Demasiadas revisiones de código se fusionan sin ningún comentario.

Me parece que no existe una revisión de código sin un solo comentario.

¿Cómo puedo, como líder del equipo, monitorear adecuadamente que mi equipo está realizando un proceso de revisión de código adecuado y cómo puedo ayudarlos a maximizar los beneficios del proceso?

Actualizar

Pensé que las personas querrían saber sobre cualquier actualización. Intenté muchas sugerencias que se dieron aquí. la mayoría ya estaban en uso. algunos ayudaron un poco. Sin embargo, el problema persistía: algunas personas continuamente recibían códigos incorrectos cuando yo no estaba buscando.

Descubrí que la supervisión de la revisión del código no es tan útil como proporcionarle a mi equipo herramientas para mejorar su código.

Así que agregué una biblioteca llamada "jscpd" para detectar las pastas de copia. La compilación falló en las pastas de copia. Eso eliminó un problema de inmediato.

A continuación vamos a probar codeclimate.

También estoy haciendo una revisión manual de revisiones de código antiguas una vez al sprint durante medio día. Estoy convirtiendo todos en problemas / tickets, como descubrí que la gente los escribe, pero nunca se manejan en un momento posterior. También estoy haciendo reuniones con todos los equipos para revisar el código cuando sea apropiado.

en general parece que nos estamos moviendo en la dirección correcta.

chico mograbi
fuente
1
En caso de que esté utilizando TFS, puede configurarlo para incorporar el Nombre del revisor de código.
Krishnandu Sarkar
11
@gnat no estoy de acuerdo. Hay una diferencia entre alguien a quien no le gustan las revisiones de código y sobre qué está preguntando esta pregunta. Esta pregunta puede ser atacada desde una perspectiva de trazabilidad (vinculando cambios en el código fuente a la revisión, o defectos / mejoras / historias a revisiones de esa implementación, etc.) o desde una perspectiva de calidad de proceso y auditoría. Ambos tienen implicaciones, incluso si las personas generalmente no tienen problemas para hacer la revisión del código.
Thomas Owens
3
¿Asiste a alguna de estas críticas? Tal vez es hora de caer en uno? Señale algunas cosas usted mismo y pregunte a cada revisor individualmente por qué se las perdió todas.
Mawg
2
¿Le parece que la revisión no ha detectado problemas obvios? Tendría que haber añadido los comentarios (importantes)?
usr

Respuestas:

70

Voy a ofrecer una opinión diferente de mis compañeros de respuesta. Tienen razón: participe si quiere ver cómo van las cosas. Si desea más trazabilidad, existen herramientas para eso.

Pero en mi experiencia, sospecho que está sucediendo algo más.

¿Has considerado que tu equipo puede sentir que el proceso está roto / estúpido / ineficaz para la mayoría de los commits? Recuerde, el proceso es documentar lo que funciona bien, no reglas para obedecer . Y a medida que el equipo dirige, usted está allí para ayudarlos a ser lo mejor posible, no para hacer cumplir las reglas.

Entonces, en sus retrospectivas (si es ágil) o uno a uno (si es un gerente) o en reuniones improvisadas en el pasillo (si es un líder de equipo no ágil y hay otro gerente haciendo uno por uno) . Pregunte qué piensa la gente sobre el proceso de revisión del código. Como esta funcionando Como no es Digamos que piensas que tal vez no está beneficiando al equipo tanto como podría. Asegúrate de escuchar .

Puede abogar por las revisiones de código en estas reuniones, pero es mejor escuchar los comentarios. Lo más probable es que descubra que su equipo piensa que es necesario ajustar el proceso "adecuado" o que hay una causa raíz (presión de tiempo, falta de revisores, Bob simplemente confirma su código, entonces ¿por qué no podemos hacerlo?) .

Forzar una herramienta sobre un proceso roto no mejorará el proceso.

Telastyn
fuente
55
+1 por el enfoque correcto para este (¡y muchos otros!) Problema
Olivier Dulac
77
+1 para la última oración. Esto es algo que casi nadie entiende, pero es extremadamente importante.
JohnEye
1
Buena respuesta. Intenté que ... Mi equipo dice "la compañía está haciendo las cosas de la manera incorrecta. Necesitamos más qa ... y dejamos que los desarrolladores se desarrollen", mientras que la compañía dice "queremos que los desarrolladores envíen código de buena calidad. Nuestro objetivo es dispersar al equipo qa dado que una vez que el código es de buena calidad, ya no se necesitan qa ... "... Lo que sucedió eventualmente es que las personas que obtuvieron un código incorrecto fueron despedidas continuamente y yo reconstruí mi equipo.
Guy mograbi
43

No me gusta publicar respuestas de una línea, pero esta parece apropiada:

Participa en el proceso.

Blrfl
fuente
15
También no me gustan las respuestas de una línea. Afortunadamente, tomaste dos líneas y mi respuesta. +1
Mawg
1
Estoy. pero cuando no lo estoy ... pasan cosas. eso es exactamente lo que me hizo sospechar en primer lugar. Empecé a volver a revisar la revisión de otros, y descubrí cosas desagradables.
Guy mograbi
6

Obtenga una herramienta, como ReviewBoard o el complemento de revisión de código de Redmine . Luego, cada revisión se crea como una tarea que debe ser cerrada o comentada por alguien (como un boleto de error). Luego puede rastrear quién creó el ticket de revisión y quién lo cerró. Puede vincular tickets de revisión con registros de código fuente, es decir, crear el ticket a partir de una revisión.

gbjbaanb
fuente
2

Algunas cosas (para ser sincero, la mayoría de ellas están cubiertas en las respuestas, pero quería ponerlas en un solo lugar)

  • Puede implementar procesos y reglas para asegurarse de que se realice una revisión de código, pero es bastante imposible ponerlos de modo que la revisión de código sea en realidad más que un ejercicio de marcación de casillas. En última instancia, el equipo tiene que ver el beneficio del proceso, para poder abordarlo de manera útil.

  • Predicar con el ejemplo. Participa en las reseñas. Como desarrollador, me siento mal si mi gerente (que ahora no es desarrollador) detecta cosas que no veo. Resalte los problemas que deberían haberse detectado en la revisión (sin culpar). Si ocurre un problema de producción, si surgen problemas durante el control de calidad (si tiene un proceso de control de calidad por separado), resalte dónde podrían haberse detectado en la revisión de código. Discuta con el equipo cómo podemos asegurar que se atrapen problemas futuros como ese

  • Discuta con el equipo lo que quieren que haga el proceso. Si no ven ningún punto (como puede suceder al principio), use los problemas de producción y los refijos necesarios como evidencia de su beneficio

  • Use un software de verificación de código automatizado como Sonarqube para que las revisiones de código puedan centrarse en problemas como código incomprensible, errores lógicos, falta de documentación, etc. que no se pueden detectar automáticamente.

monstruo mate
fuente
2

Puede documentar lo que el equipo quiere en las revisiones de código que ha discutido y acordado con los desarrolladores. Algunas cosas que podría considerar como parte de las revisiones de código son:

  • Compruebe que el código hace lo que se supone que debe hacer, es decir, cumple los requisitos

  • Estilo de código para garantizar que los desarrolladores estén codificando un estilo coherente

  • Optimización, por ejemplo, número de llamadas a funciones

  • Arquitectura y reutilización

  • Manejo y registro de excepciones.

  • Deuda técnica: el código está en mejor estado que cuando el desarrollador comenzó a trabajar en él

  • Echa un vistazo y crea el código (me parece útil, pero otros desarrolladores de mi equipo prefieren dejar esto a los evaluadores)

  • Usando una herramienta automatizada (he usado SonarQube ). Me resulta útil integrar esto en su proceso de compilación para aplicar mejoras al código, por ejemplo, aumentar la cobertura de prueba

Algunos de los pasos anteriores pueden ser cubiertos por una herramienta automatizada, pero mientras intentas mejorar la forma en que se realizan o revisan los códigos, probablemente valga la pena usar tanto la herramienta como la revisión del globo ocular. Sin embargo, los pasos más importantes para prevenir la deuda técnica (arquitectura y reutilización) no pueden ser totalmente automatizados.

Si su equipo es inconsistente en la aplicación de esto, podría intentar solo permitir que los desarrolladores que realizan revisiones de código correctamente tengan derechos de fusión. Por ejemplo, es posible que solo desee comenzar con el desarrollador principal del equipo. La compensación con este enfoque es que esos desarrolladores podrían convertirse en un cuello de botella en el proceso de desarrollo, por lo que usted y el equipo deben decidir si lo desean. Personalmente, aceptaría esta compensación y, a través de las revisiones de código, aumentaría la disciplina en el resto del equipo y luego, cuando esté listo, puede aumentar el número de desarrolladores con derechos de fusión.

Finalmente, vale la pena revisar las reseñas. Entonces, una vez a la semana, reúnanse con los desarrolladores y discutan constructivamente las revisiones y las formas de mejorarlas.

br3w5
fuente
¿Es este un anuncio para SonarQube? Lo probé, no lo recomendaría, demasiado doloroso para comenzar y aunque el "código abierto" cuesta todos los bits útiles.
gbjbaanb
Está funcionando bien en mi equipo actual y no fue demasiado difícil de configurar y está ayudando: no es un anuncio, pero es la única herramienta de este tipo de la que tengo experiencia. ¿Diría lo mismo para Redmine codereview y ReviewBoard?
br3w5
Estamos utilizando SonarQube en nuestros equipos, atendiendo alrededor de 70 proyectos, que van desde 10k hasta 3M LOC. Aunque algunos equipos simplemente ignoran sus informes, la mayoría lo usa para dirigir procesos de refactorización. Funciona bien, aunque personalmente prefiero herramientas simples y no integradas, como Findbugs.
Dibbeke
Y aquí estaba yo pensando que la revisión del código implicaba verificar si el código coincidía con el documento de diseño: - /
Mawg
1
Gracias, esto es lo que estoy haciendo mientras tanto. Actualizaré en un par de semanas cómo afectó.
Guy Mograbi
0

Te diré cómo mi equipo integró rápidamente la revisión de código en su flujo de trabajo.

Primero, déjame hacerte una pregunta. ¿Está utilizando un sistema de control de versiones (por ejemplo, Mercurial, Git)?

Si su respuesta es sí, entonces proceda.

  1. prohibir a todos empujar cualquier cosa (incluso pequeños arreglos) directamente a la rama maestra (troncal) *
  2. desarrollar nuevas características (o arreglos) en ramas separadas
  3. cuando los desarrolladores creen que la sucursal está lista para integrarse en master, crearán una "solicitud de extracción"
  4. prohibir a todos fusionar su propia solicitud de extracción *
  5. haga que otro desarrollador evalúe la solicitud de extracción y revise el nuevo código
  6. si el código pasa la revisión, bien, la solicitud de extracción se puede fusionar, de lo contrario se pueden aplicar correcciones
  7. repita el paso 6 hasta que el código madure lo suficiente (se puede hacer sin comenzar de nuevo) **
  8. hecho, todo su nuevo código se revisa (al menos sumariamente), por alguien con un nombre

Ahora tiene un punto preciso en su flujo de trabajo donde se realiza la revisión del código.

Actúa allí.

* se puede aplicar automáticamente, con ganchos del lado del servidor

** este procedimiento es totalmente compatible con GitHub (entre otros) y es bastante fácil de usar, échale un vistazo

Agostino
fuente
2
Incluso con dicho proceso (que supuse que realmente sucede a partir de la descripción en la pregunta), a veces los desarrolladores piensan "ah, confío en mi colega lo suficiente y tengo mucho que hacer, así que lo fusionaré sin leerlo realmente los detalles, o incluso comentarlo ". (Tenemos un proceso similar en nuestro equipo, con dos aprobaciones necesarias (de personas que no sean el autor de relaciones públicas), antes de que pueda fusionarse. Sin embargo, a veces los cambios se realizan sin una revisión exhaustiva.)
Paŭlo Ebermann
1
@ PaŭloEbermann ya veo. Me temo que es un resultado inevitable de las circunstancias, si no tiene suficiente tiempo para hacer todo lo que necesita, la calidad se verá afectada, de una forma u otra. Sin embargo, si no funciona "a veces", eso significa que funciona "la mayor parte del tiempo", ¿no?
Agostino
1
Sí, ayudó un poco al permitir la fusión solo para un conjunto limitado de personas, que tenían la tarea de verificar si la revisión real se realizó correctamente.
Paŭlo Ebermann
Tenía una prohibición similar, y no hace falta decir que el desarrollo casi se detuvo. Esa regla duró 2 semanas enteras, después de lo cual los gerentes tuvieron que ajustar sus planes.
B 11овић
@ BЈовић ¿su equipo hacía revisiones de código regularmente antes ? Muchos utilizan esta técnica, especialmente en el ecosistema de código abierto. El hecho de que no funcionó para su equipo no significa que no pueda funcionar para otros.
Agostino
-2

Creo que debería crear una plantilla y pedirles a los miembros de su equipo que la actualicen cada vez que revisen el código. Pero incluso entonces, debe participar en el proceso de revisión inicialmente.

usuario85
fuente