¿Cómo suele suceder una revisión de código? Envía una solicitud de grupo, recibe comentarios, realiza correcciones, envía arreglos para una segunda revisión, luego obtiene la aprobación y se produce la fusión. Suena simple, pero en realidad el proceso de revisión puede llevar mucho tiempo.
Imagine que tiene una solicitud de grupo con cientos de líneas de cambio. El revisor debe dedicar mucho tiempo a leer completamente el código y comprender los cambios propuestos. Como resultado, todo el proceso desde la creación de una solicitud de grupo hasta su aprobación puede llevar varios días; esto no es muy agradable tanto para el revisor como para el autor de los cambios. Y hay muchas posibilidades de que, al final, el crítico aún se pierda algo. O la verificación puede ser demasiado superficial y, en el peor de los casos, la solicitud de agrupación puede rechazarse de inmediato.
Resulta que cuanto mayor sea la solicitud del grupo, menor será el beneficio de verificarla.
¿Cómo evitar tales situaciones? ¿Cómo hacer que la solicitud de grupo sea más fácil y más comprensible para el revisor y optimizar todo el proceso?
Estamos traduciendo un artículo de nuestro desarrollador backend Sergey Zhuk sobre cómo funciona el proceso de revisión de código del equipo de aplicaciones móviles Skyeng.
Cambiar categorías
Imaginemos que tiene una tarea: implementar una nueva funcionalidad en el proyecto. La solicitud de grupo en la que está trabajando puede contener diferentes categorías de cambios. Por supuesto, habrá algún código nuevo en él. Pero en el curso del trabajo, puede notar que algunos códigos deben ser refactorizados de antemano para que contribuyan a la adición de nuevas funcionalidades. O con esta nueva funcionalidad, la duplicación ha aparecido en el código que desea eliminar. O de repente encontraste un error y quieres arreglarlo. ¿Cómo debería ser la solicitud de grupo final?
Primero, veamos qué categorías de cambios pueden ocurrir con el código.
- Cambios funcionales
- Refactorización estructural: cambios en clases, interfaces, métodos, movimiento entre clases.
- Refactorización simple: se puede realizar utilizando el IDE (por ejemplo, extracción de variables / métodos / constantes, simplificación de condiciones).
- Renombrar y mover clases: reorganizar el espacio de nombres.
- Eliminando el código no utilizado (muerto).
- Código de correcciones de estilo.
Revisión de estrategia
Es muy importante comprender que cada una de estas categorías se revisa de manera diferente y, al considerarlas, el revisor debe centrarse en cosas diferentes. Se puede decir que cada categoría de cambio implica su propia estrategia de revisión.
- Cambio funcional: resolución de problemas empresariales y diseño de sistemas.
- Refactorización estructural: compatibilidad con versiones anteriores y mejora del diseño.
- Refactorización primitiva: legibilidad mejorada. Estos cambios se pueden realizar principalmente utilizando el IDE (por ejemplo, extracción de variables / métodos / constantes, etc.).
- Renombrar / mover clases: ¿ha mejorado la estructura del espacio de nombres?
- Eliminación de código no utilizado: compatible con versiones anteriores.
- Estilo del código de correcciones: la solicitud de agrupación de fusión más frecuente ocurre inmediatamente.
Se utilizan diferentes enfoques para verificar los cambios en diferentes categorías, por lo tanto, la cantidad de tiempo y esfuerzo dedicado a su revisión también varía.
Cambios funcionales Este es el proceso más largo porque implica cambiar la lógica del dominio. El revisor busca si el problema está resuelto y verifica si la solución propuesta es la más adecuada o si se puede mejorar.
Refactorización estructural. Este proceso requiere mucho menos tiempo que los cambios funcionales. Pero puede haber sugerencias y desacuerdos sobre cómo debe organizarse exactamente el código.
Al verificar las categorías restantes en el 99% de los casos, la fusión ocurre inmediatamente.
- Refactorización simple. ¿Se ha vuelto más legible el código? - Merzhim.
- Renombrar / mover clases. ¿Se ha trasladado la clase a un mejor espacio de nombres? - Merzh.
- Eliminar el código no utilizado (muerto) es merzhim.
- Estilo de código de correcciones o formato - merzh. Sus colegas no deben verificar esto durante la revisión del código, esta es la tarea de la interfaz.
¿Por qué deberían clasificarse los cambios?
Ya hemos discutido que las diferentes categorías de cambios se revisan de manera diferente. Por ejemplo, verificamos los cambios funcionales basados en los requisitos del negocio, y en la refactorización estructural, verificamos la compatibilidad con versiones anteriores. Y si mezclamos varias categorías, será difícil para el revisor tener en cuenta varias estrategias de revisión al mismo tiempo. Y, lo más probable, el revisor pasará más tiempo en la solicitud del grupo de lo necesario, y debido a esto, puede perderse algo. Además, si la solicitud del grupo contiene varios tipos de cambios, con cualquier corrección, el revisor tendrá que revisar todas estas categorías nuevamente. Por ejemplo, mezcló refactorización estructural y cambios funcionales. Incluso si la refactorización se realiza bien, pero hay un problema con la implementación de lo funcional, entonces, después de las correcciones, el revisor tendrá que mirar toda la solicitud del grupo desde el principio. Es decir, vuelva a verificar tanto la refactorización como los cambios funcionales. Por lo tanto, el revisor dedica más tiempo a la solicitud del grupo. En lugar de contemplar inmediatamente una solicitud de agrupación separada con refactorización, el revisor debe revisar nuevamente el código completo.
Lo que no vale la pena mezclar exactamente
Renombrar / eliminar una clase y su refactorización. Aquí nos encontramos con Git, que no siempre comprende correctamente dichos cambios. Me refiero a los cambios a gran escala cuando cambian muchas líneas. Cuando refactoriza una clase y luego la mueve a alguna parte, Git no la percibe como en movimiento. En cambio, Git interpreta estos cambios como eliminar una clase y crear otra. Esto lleva a un montón de preguntas durante la revisión del código. Y se le pregunta al autor del código por qué escribió este código feo, aunque en realidad este código simplemente se movió de un lugar a otro con cambios menores.
Cualquier cambio funcional + cualquier refactorización. Ya hemos discutido este caso arriba. Esto hace que el revisor tenga en cuenta dos estrategias de revisión a la vez. Incluso si la refactorización se realiza bien, no podremos retrasar estos cambios hasta que se aprueben los cambios funcionales.
Cualquier cambio mecánico + cualquier cambio realizado por el hombre. Por "cambios mecánicos" me refiero a cualquier formateo realizado utilizando el IDE o la generación de código. Por ejemplo, aplicamos el nuevo estilo de código y obtenemos 3000 cambios de línea. Y si mezclamos estos cambios con cualquier cambio funcional o de otro tipo realizado por una persona, obligaremos al revisor a clasificar mentalmente estos cambios y la razón: este es un cambio realizado por una computadora; puede omitirse, y este cambio es necesario. echa un vistazo Por lo tanto, el revisor dedica mucho tiempo extra a la verificación.
Ejemplo
Aquí hay una solicitud de grupo con una función de método que cierra suavemente la conexión del cliente a Memcached:
Incluso si la solicitud del grupo es pequeña y contiene solo cien líneas de código, aún es difícil de verificar. Como puede ver en las confirmaciones, contiene varias categorías de cambios:
- funcional (nuevo código),
- refactorización (creación / movimiento de clases),
- estilo de código de correcciones (eliminación de bloques de muelle en exceso).
Al mismo tiempo, la nueva funcionalidad en sí es de solo 10 líneas:
Como resultado, el revisor debe revisar todo el código y
- Verifique que la refactorización esté bien.
- compruebe si la nueva funcionalidad se implementa correctamente;
- determine si este cambio fue generado automáticamente por el IDE o por la persona.
Por lo tanto, dicha solicitud de grupo es difícil de revisar. Para corregir la situación, puede dividir estas confirmaciones en ramas separadas y crear un grupo de solicitudes para cada una de ellas.
1. Refactorización: extracción de clase
Solo hay dos archivos. El revisor solo debe verificar el nuevo diseño. Si todo está en orden - merzhim.
2. El siguiente paso también es refactorizar, simplemente movemos las dos clases a un nuevo espacio de nombres
Dicha solicitud de grupo es bastante simple de verificar; se puede instanciar de inmediato.
3. Eliminar el exceso de bloques de muelle
Nada interesante aquí Merzhim
4. El funcional en sí
Y ahora la solicitud de grupo con cambios funcionales contiene solo el código deseado. Por lo tanto, su revisor puede centrarse solo en esta tarea. La solicitud de la piscina es pequeña y fácil de verificar.
Conclusión
Regla de oro:No cree grandes solicitudes de grupo con categorías mixtas de cambios.
Cuanto mayor sea la solicitud del grupo, más difícil será para el revisor comprender los cambios que ha propuesto. Lo más probable es que se rechace una solicitud de grupo enorme con cientos de líneas de código. En cambio, divídalo en pequeñas partes lógicas. Si su refactorización está en orden, pero los cambios funcionales contienen errores, entonces la refactorización puede retenerse fácilmente, por lo que usted y su revisor pueden centrarse en la funcionalidad sin mirar todo el código desde el principio.
Y siempre siga estos pasos antes de enviar la solicitud de grupo:
- Optimiza tu código para leer. El código es mucho más legible que escrito;
- Describa los cambios propuestos para proporcionar el contexto necesario para su comprensión;
- siempre verifique su código antes de crear una solicitud de grupo. Y hazlo como si fuera el código de otra persona. A veces ayuda encontrar algo que te perdiste. Esto reducirá la probabilidad de rechazo de su solicitud de grupo y el número de correcciones.
Mi colega
Oleg Antonevich me habló de la idea de dividir los cambios en categorías.
Del Editor: Sergey escribe muchas cosas interesantes sobre programación y PHP, y a veces traducimos algo: un servidor de video en tiempo real , renderizando archivos HTML . Siéntase libre de hacerle preguntas en los comentarios a este artículo, ¡él responderá!
¡Bien, también le recordamos que siempre tenemos muchas vacantes interesantes para desarrolladores!