Cómo hacer revisiones de código más rápido y más eficiente

imagen

¬Ņ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.

  1. Cambios funcionales
  2. Refactorización estructural: cambios en clases, interfaces, métodos, movimiento entre clases.
  3. Refactorización simple: se puede realizar utilizando el IDE (por ejemplo, extracción de variables / métodos / constantes, simplificación de condiciones).
  4. Renombrar y mover clases: reorganizar el espacio de nombres.
  5. Eliminando el código no utilizado (muerto).
  6. 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.

  1. Cambio funcional: resoluci√≥n de problemas empresariales y dise√Īo de sistemas.
  2. Refactorizaci√≥n estructural: compatibilidad con versiones anteriores y mejora del dise√Īo.
  3. Refactorización primitiva: legibilidad mejorada. Estos cambios se pueden realizar principalmente utilizando el IDE (por ejemplo, extracción de variables / métodos / constantes, etc.).
  4. Renombrar / mover clases: ¬Ņha mejorado la estructura del espacio de nombres?
  5. Eliminación de código no utilizado: compatible con versiones anteriores.
  6. 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.

  1. Refactorizaci√≥n simple. ¬ŅSe ha vuelto m√°s legible el c√≥digo? - Merzhim.
  2. Renombrar / mover clases. ¬ŅSe ha trasladado la clase a un mejor espacio de nombres? - Merzh.
  3. Eliminar el código no utilizado (muerto) es merzhim.
  4. 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:

imagen

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:

imagen

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


imagen

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


imagen

Dicha solicitud de grupo es bastante simple de verificar; se puede instanciar de inmediato.

3. Eliminar el exceso de bloques de muelle


imagen

Nada interesante aquí Merzhim

4. El funcional en sí


imagen

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!

Source: https://habr.com/ru/post/443402/


All Articles