¿Cómo hacer mejores revisiones de código cuando las solicitudes de extracción son grandes?

12

Descargo de responsabilidad: hay algunas preguntas similares, pero no encontré ninguna que tocara específicamente los problemas que enfrenta mientras revisaba una gran solicitud de extracción.

Problema

Siento que mis revisiones de código podrían hacerse de una mejor manera. Estoy hablando particularmente de revisiones de códigos grandes con muchos cambios en más de 20 archivos.

Es bastante simple detectar problemas obvios de código local. Sin embargo, comprender si el código cumple con los criterios comerciales es una historia diferente.

Tengo problemas para seguir el proceso de pensamiento del autor del código. Es bastante difícil cuando los cambios son numerosos y se extienden a través de múltiples archivos. Intento concentrarme en los grupos de archivos relacionados con una pieza particular de cambios. Luego revise los grupos uno por uno. Lamentablemente, la herramienta que uso (Atlassian Bitbucket) no es muy útil. Cada vez que visito un archivo, se marca como se ve, a pesar de que a menudo resulta que no está relacionado con la pieza de cambios examinada actualmente. Sin mencionar que algunos archivos deben ser visitados varias veces y sus cambios revisados ​​pieza por pieza. También volver a los archivos relevantes cuando sigues un mal camino no es fácil.

Posibles soluciones y por qué no funcionan para mí.

Revisar una solicitud de extracción mediante commits a menudo resuelve los problemas de tamaño, pero no me gusta, ya que frecuentemente veré cambios obsoletos.

Por supuesto, crear solicitudes de extracción más pequeñas parece un remedio, pero es lo que es, a veces se obtiene una gran solicitud de extracción y hay que revisarla.

También puede ignorar el aspecto lógico del código en su conjunto, pero parece bastante arriesgado, especialmente cuando el código proviene de un programador inexperto.

Usar una herramienta mejor podría ser útil, pero no encontré una.

Preguntas

  • ¿Tiene problemas similares con sus revisiones de código? ¿Cómo los enfrentas?
  • ¿Quizás tienes mejores herramientas?
Andrzej Gis
fuente
3
¿Por qué son tan grandes las revisiones de código? Por ejemplo, si son el resultado de una refactorización automática, entonces, en lugar de revisar la confirmación, verifica si repetir la refactorización en la confirmación anterior produce una copia idéntica de la nueva confirmación y luego decide si confía en la herramienta o no. Entonces, revisar un diferencial de 1000 líneas de repente se convierte en revisar un comando de 1 línea en un IDE y en una decisión de confiar en el proveedor de IDE.
Jörg W Mittag
Si tiene la capacidad de hacerlo, hágalo responsabilidad del autor del código para facilitar la revisión del código. Esto significa que los autores deben ser conscientes de aplastar los compromisos irrelevantes, reescribir los compromisos para que contengan solo un cambio, separar los principales compromisos de refactorización y ordenar los compromisos de una manera que tenga sentido para los revisores.
Mentira Ryan el

Respuestas:

8

Tuvimos estos problemas y hacer la siguiente pregunta ha funcionado bien para nosotros:

¿El RP hace una cosa? que se puede fusionar y se puede probar de forma independiente?

Tratamos de romper los RP por responsabilidad única (RS). Después del retroceso inicial, la gente se sorprendió al descubrir que incluso algo pequeño, aunque solo, puede ser grande.

El SR hace que sea realmente fácil de revisar y también difunde el conocimiento de la implementación esperada.

¡Esto también permite refactores incrementales a medida que se agrega más y el tiempo de respuesta de PR se reduce drásticamente!

Sugeriría dividirlos por SR si es posible y ver si funciona para usted. Toma algo de práctica hacerlo de esa manera.

Doctor
fuente
11

A veces no puede evitar grandes solicitudes de extracción, pero puede discernir quién tiene la responsabilidad.

Trato las solicitudes de extracción como argumentos persuasivos. El autor está tratando de convencerme de que el código debe verse y funcionar de esta manera.

Como con cualquier argumento, debe tener una sola idea clara. Ya sea:

  • un refactor,
  • una optimización,
  • o nueva funcionalidad.

Si no están siendo claros, entonces hay una buena posibilidad de que no lo entiendan ellos mismos. Abre el diálogo y ayúdalos a dividir su argumento en subargumentos. Si es necesario, está perfectamente bien, incluso es beneficioso para ellos recrear esos commits y ofrecer solicitudes de extracción más comprensibles y directas.

Todavía habrá grandes solicitudes de extracción, pero con un argumento claro es mucho más fácil ver lo que no encaja.

En cuanto a las herramientas, depende de su organización y proceso. BitBucket es una herramienta decente, ya sea que sea mejor o no, depende de todo, desde su presupuesto, hardware, requisitos, hasta sus procesos preexistentes, reglas comerciales y varias adaptaciones de software que ya ha realizado para acomodar BitBucket. Comenzaría por revisar la documentación para ver si el comportamiento se puede configurar, tal vez tirarlo a la comunidad de complementos (o unirlo creando un complemento para hacer eso).

Kain0_0
fuente
8

No revise la solicitud de extracción completa, sino cada confirmación. Obtendrá una mejor comprensión de la solicitud de extracción de todos modos al hacerlo de esta manera.

Si las confirmaciones son pequeñas, hacer esa revisión no debería ser un problema. Sigue los cambios uno por uno a través de las confirmaciones y termina obteniendo la imagen completa. Hay inconvenientes, como que a veces revisará los cambios que se deshacerán algunas confirmaciones más adelante, pero esto no debería ser una gran oferta.

Si los commits son grandes, entonces debe discutirlo con la persona que hizo esos commits. Explíquele por qué los grandes commits son problemáticos. Escuche los argumentos de la otra persona acerca de por qué rara vez comete cambios: puede aprender, por ejemplo, que evita hacer compromisos porque los ganchos previos al compromiso tardan demasiado, en cuyo caso este problema debe resolverse primero.

Revisar una solicitud de extracción mediante commits a menudo resuelve los problemas de tamaño, pero no me gusta, ya que frecuentemente veré cambios obsoletos.

Lo haces, pero este es un problema menor, y deberías perder mucho menos tiempo revisando el código, que se deshará algunas confirmaciones más tarde que el tiempo que malgastas tratando de descubrir por qué se cambió el código al revisar un solo archivo.

"Frecuentemente" es vago, pero si se encuentra pasando demasiado tiempo revisando el código que no llegó a la revisión final de la solicitud de extracción, puede usar dos técnicas:

  • Revisa rápidamente todos los commits una vez, solo lee los mensajes de commit. De esta manera, cuando estudies un commit en particular, puedes recordar que un mensaje de commit en algún lugar más tarde dijo que el cambio que estás viendo fue revertido.

  • Tenga una vista lado a lado de la última versión del archivo (aunque en muchos casos, para grandes confirmaciones, (1) los archivos pueden ser radicalmente diferentes y (2) los archivos pueden ser renombrados o los grandes bloques de código pueden se trasladará a otro lugar, lo que dificulta encontrar el archivo correspondiente).

  • Solicite a los encargados de la confirmación que eliminen las confirmaciones cuando tenga sentido, o tenga convenciones específicas de mensajes de confirmación en las que una confirmación deshaga una parte de otra, y haga su revisión teniendo en cuenta varias confirmaciones siguientes.


¹ Por ejemplo, imagine que está viendo un archivo donde se ha cambiado el nombre de alguna variable. ¿Qué te dice? ¿Cómo debería revisar este cambio? Sin embargo, si lo estuviera revisando commit por commit, vería que un solo commit renombró la misma variable en veinte archivos, y el comentario indica que el nombre se cambió para usar el término comercial adecuado. Este cambio tiene mucho sentido y es posible revisarlo.

Arseni Mourzenko
fuente
1
@ JörgWMittag: gracias por tu comentario. El OP afirmó que no quiere hacer revisiones por confirmación porque vería cambios obsoletos, lo cual es cierto, pero no debería ser tan problemático como tener todos los problemas relacionados con la revisión por archivo. Como mi respuesta no estaba clara al respecto, agregué una sección para abordar específicamente este punto.
Arseni Mourzenko
2

Calcule lo que está tratando de lograr con revisiones de solicitudes de extracción y vea si hay una alternativa.

Por ejemplo, es posible que desee

  • Asegurar que se sigan los estándares
  • Verifique que la funcionalidad sea correcta
  • Asegúrese de que más de una persona entienda el código
  • Entrena juniors

etcétera etcétera.

Algunos de estos podrían ser mejor atendidos por otras cosas, e incluso solo entender las razones le permite limitar el alcance de sus cheques.

Por ejemplo, si estoy revisando cada línea para poder sugerir y discutir los cambios para la capacitación, entonces puedo omitir eso en un RP grande hecho por personas mayores

Si necesito entender el código, quizás empareje la programación en funciones grandes y limite la revisión del código a una verificación de estándares.

No necesita verificar todas las cosas en cada RP siempre y cuando tenga un enfoque por capas para el control de calidad

Ewan
fuente