Hacer que TODOS los desarrolladores hagan revisiones de código

13

Soy desarrollador de software en un equipo de desarrolladores de 7-8. Hemos estado haciendo revisiones de código durante bastante tiempo y la calidad del código ha mejorado con el tiempo.

Sin embargo, recientemente noté que a algunos desarrolladores se les pide más revisiones de código que a otros. Me temo que es por su actitud flexible.

En mi opinión, no es así como deberían realizarse las revisiones de código: todo el equipo debería ser responsable de ello y los revisores de código no deberían ser elegidos por su disposición a aceptar fácilmente los cambios.

¿Cómo manejas este problema en tu equipo?

¿Ha establecido reglas para elegir revisores de código?

¿Crees que los revisores de código deberían ser recompensados ​​por el tiempo que pasan haciendo revisiones de código (buenas)? ¿Y cómo deberían ser recompensados?

Gracias por sus respuestas / ideas.

guillaumegallois
fuente
77
¿Ha considerado crear un sistema round robin en el que tanto el codificador se quede en la oscuridad sobre quién está revisando y el revisor se quede en la oscuridad sobre quién es el codificador?
Neil
1
¡No lo he hecho, pero me gusta esta idea! ¡Gracias!
guillaumegallois
1
¿Quién está a cargo y por qué no hacen su trabajo, lo que debería implicar asegurarse de que todos los demás hagan el suyo?
JeffO
En mi equipo, los revisores se asignan automáticamente cada vez que se abre una solicitud de extracción. Los revisores son seleccionados del equipo de todos contra todos. Tenemos un webhook para cada uno de nuestros repositorios que asigna revisores automáticamente cada vez que se abre el RP. Básicamente mantiene una lista de todos los desarrolladores, y quién fue el último asignado, y simplemente se abre paso a través de la lista.
Dan Jones

Respuestas:

12

No elegimos revisores.

En nuestro equipo:

  • Todos los cambios de código deben revisarse antes de completar la solicitud de extracción
  • Al menos un desarrollador debe revisar el código (dos o más revisores son mejores y tener probadores, analistas y otros miembros del equipo haciendo la revisión del código también es extremadamente beneficioso)

Las revisiones de código no se asignan, las personas las recogen cuando les funciona. El entendimiento es que no podemos impulsar las historias a través de la tubería. En ocasiones, alguien mencionará que están esperando un CR en el standup, pero eso es todo.

Me gusta este modelo, le da a la gente lo que puede y evita "darle trabajo a la gente".

Liath
fuente
6

Introduzca una regla que indique que se puede asignar un error para corregirlo, no solo a quienes cometieron el cambio, sino solo a quienes lo revisaron. Eso debería crear una perspectiva correcta para la revisión del código.

Como para,

¿Crees que los revisores de código deberían ser recompensados ​​por el tiempo que pasan haciendo revisiones de código (buenas)?

Bueno, no estoy seguro de cómo generalmente los desarrolladores son recompensados ​​por hacer su trabajo, excepto solo recibir un salario y estar orgullosos de lo que han hecho. Pero como el código de revisión es parte de su trabajo, el revisor debe tener tiempo para la revisión y compartir el crédito de alguna manera.

max630
fuente
2
Es una idea interesante, pero muchas veces, cuando surge un error, el 90% del trabajo está calculando exactamente qué está causando el error, y el 10% del trabajo lo está solucionando. Hacer una autopsia para descubrir exactamente qué cambio introdujo el error puede que ni siquiera suceda, a menos que ayude a descubrir qué está sucediendo o cómo hacer una solución segura.
DaveG
Usted hizo un buen comentario sobre el crédito que se les debe otorgar a los revisores de código. Este es definitivamente un problema que debe abordarse. ¡Gracias por tu respuesta!
guillaumegallois
Creo que podría hacer que la gente intente no hacer revisiones de código en absoluto o podría ser que no se realizará ningún trabajo porque la gente comenzará a hacer trampas.
Mateusz
55
Esta respuesta asume que el objetivo de las revisiones de código es encontrar errores. Ese no es el caso, el objetivo principal es mantener el código en un estado mantenible y evolutivo (y si tiene suerte, también se encuentran algunos errores).
Doc Brown
@DocBrown para evitar errores y también para asegurarse de que la corrección futura del error sea más rápida (tanto familiarizando al otro desarrollador con el código como asegurándose de que el código esté bien escrito)
max630
4

El principal problema que tiene no es técnico, pero algunas herramientas pueden reducir la cantidad de esfuerzo que requieren las revisiones de código. Hay algunos desafíos que superar:

  • Comprender cuál es el cambio. Las solicitudes Git Pull en las ramas de características realmente ayudan a este proceso.
  • Hacer que el código revise un evento donde las personas deben estar en la misma habitación. Esto agrega el estrés de la programación, los recursos para reuniones, etc. GitHub, GitLab y BitBucket permiten revisiones asincrónicas para que puedan ocurrir cuando el par está listo.
  • La capacidad de proporcionar comentarios significativos cuando se mira el código. Para ser honesto, la función de comentarios línea por línea en GitHub, GitLab, las solicitudes de extracción de Bitbucket son realmente más útiles que una reunión cara a cara. Se siente menos político.

Eso no quiere decir que no pueda usar SubVersion u otras herramientas (como Fisheye) para ayudar, pero la integración integrada en la tubería de Git con ramas de características realmente hace que el trabajo sea menos complicado.

Fuera de las herramientas, debe considerar más desafíos sociales:

  • Haga que sus desarrolladores comiencen su día revisando cualquier solicitud de extracción abierta para cerrar sesión.
  • Haga que sus desarrolladores revisen cualquier solicitud de extracción abierta antes de comenzar una nueva tarea
  • Requiere la aprobación de más de una persona para sus solicitudes de extracción.

También podría valer la pena verificar qué tipos de tareas están siendo revisadas por código por las personas más comprometidas. Podrían estar obteniendo todas las reseñas fáciles, lo que hace que las cosas sean más dolorosas para todos los demás.

Berin Loritsch
fuente
El último punto es bueno. Una vez trabajé con un desarrollador en un equipo pequeño que solo revisaba los cambios en el software que había escrito que causaban cuellos de botella significativos en otras partes del equipo.
Robbie Dee
En ese caso, parece que tienes a alguien tratando de proteger su "territorio". En la medida de lo posible, desea fomentar un sentido de propiedad de la comunidad para el código. En otras palabras, no hay santuarios protegidos dentro del código que ningún otro desarrollador excepto el "bendecido" pueda tocar. Entiendo si hay una brecha de especialidad y no puede revisar las matemáticas, pero al menos puede revisar qué tan bien coincide el código con su intención.
Berin Loritsch
2

Una buena idea es hacerlo de forma redonda: elige a alguien que haya realizado la menor cantidad de revisiones para su código. Con el tiempo, todos en el equipo deberían haber hecho aproximadamente el mismo número de revisiones. Difunde el conocimiento también.

Obviamente habrá excepciones ocasionales como días festivos donde habrá picos y valles.

No debe haber distinción entre juniors y seniors / leads. Las revisiones de código deben llevarse a cabo para el código de todos, sin importar cuán altos sean. Reduce la fricción y ayuda a compartir diferentes enfoques.

Si aún le preocupa la calidad de las revisiones después de todo esto, considere la introducción de un conjunto de estándares mínimos para que se apruebe una revisión de código. Lo que incluya depende totalmente de usted, pero algunas cosas que puede considerar son la cobertura del código, las pruebas unitarias, la eliminación del código comentado, las métricas, los comentarios suficientes, la calidad de construcción, los principios SÓLIDOS, DRY, KISS, etc.

En cuanto a las revisiones de código de incentivo, el código de calidad es la recompensa. Todos estamos seguros de que trabajamos en bases de código subóptimas en las que el dolor podría haber disminuido considerablemente si otro desarrollador le hubiera dado una vez más el código y sugeriera cambios constructivos.

Robbie Dee
fuente
0

Parece que el equipo carece de un proceso formal para las revisiones de código.

No estoy hablando de crear un documento de Word de 350 páginas, sino solo algunos puntos simples sobre lo que implica el proceso.

Los bits importantes:

  1. Definir un conjunto central de revisores. No hay declaraciones generales. Nombra personas.

    Estos deberían ser sus desarrolladores senior.

  2. Solicite a más de 1 revisor principal que apruebe la revisión.

  3. Identifique al menos otro revisor no principal en cada sprint o suelte quién es un revisor principal temporal. Solicite su firma en todas las revisiones de código durante este tiempo.

El elemento n. ° 3 permite que los otros desarrolladores se incorporen al grupo principal de revisores. Algunas semanas pasarán más tiempo en las revisiones que otras. Es un acto de equilibrio.

¿En cuanto a recompensar a las personas? Muchas veces reconocer el esfuerzo que una persona está haciendo durante la revisión del código en frente de todo el equipo puede funcionar, pero no se estrese por esto.

En caso de duda, defina el proceso y dígale al equipo que deben cumplirlo.

Y hay una última cosa que puedes probar, por controvertida que sea: deja que el @ # $% golpee al fan, si puedo usar un idioma.

Deje que el equipo falle, porque no se sigue el proceso de revisión del código. La gerencia se involucrará y luego la gente cambiará. Esta es realmente una buena idea en los casos más extremos en los que ya definió un proceso y el equipo se negó a cumplirlo. Si no tiene la autoridad para despedir o disciplinar a las personas (como la mayoría de los desarrolladores principales no tienen ), entonces necesita involucrar a alguien que pueda hacer esto.

Y no hay nada como no lograr que las cosas cambien. A pesar de lo que digan las personas, se puede dirigir el Titanic - pero no antes de que llegue el burgo de hielo.

A veces solo necesitas dejar que el Titanic golpee el hielo.

Greg Burghardt
fuente