Cultura de revisión de código: por qué es necesario tenerla

¿Cuántas veces cruzas los dedos al abrir una solicitud de incorporación de cambios con la esperanza de que no se le asigne a ese desarrollador de tu equipo que siempre deja un puñado de comentarios sobre todo? No me refiero a esos comentarios muy razonables que revelan errores cometidos o, por favor, no, código maloliente.

Estos son comentarios sobre el estilo de tu código o algunas cosas pequeñas y subjetivas que no tienen como objetivo mejorar la base del código al final, sino que más bien te hacen pensar: "Lo habría hecho de otra manera porque me gusta más". Como compañero de equipo responsable, es probable que los abordes.

No importa qué opción elijas (aplicar todos los cambios o contraatacar con largas explicaciones sobre por qué no estás de acuerdo con esto o aquello), probablemente terminarás sintiéndote agitado, exhausto y frustrado por el tiempo que dedicaste a cosas no esenciales. ¿Realmente valió la pena? Esta pregunta surgirá en tu cabeza cada vez que te enfrentes a este tipo de situaciones.

O, a veces, tienes el otro extremo del palo: apruebas instantáneamente sin comentarios ni señales de que un revisor haya revisado cuidadosamente tus cambios. Ya sea que lo hayan revisado y no haya aportes objetivos o no, te quedas con dudas sobre ti mismo y sobre este compañero de equipo.

Por supuesto, incluso si todo está bien, ¿no sería bueno incluir una rápida mención al autor y asegurarse de que están en la misma página sin que queden dudas?

Si las dos historias anteriores le resultan familiares, este artículo es para usted. Lo que le falta a su equipo es una cultura de solicitudes de incorporación de cambios o, en otras palabras, un conjunto de pautas sobre cómo comunicarse en las solicitudes de incorporación de cambios para respaldar un proceso de colaboración amigable y altamente eficiente. Abordaré los aspectos esenciales que los revisores querrían buscar y también ejemplos de comentarios cuestionables (o más bien, críticas quisquillosas) que generan fricción o, a veces, incluso problemas que van más allá de sus sentimientos personales.

Como desarrollador de iOS, usaré Swift en mis ejemplos de código, pero, en general, este tema es importante para cualquier desarrollador, sin importar la plataforma o el lenguaje que use a diario.

Sin embargo, creo que la cultura de revisión de código es aún más relevante para las plataformas de Apple porque, según mi experiencia, tendemos a ser más exigentes en muchos aspectos. Probablemente se deba a una mentalidad perfeccionista heredada de la visión de Steve en los viejos tiempos.

Lección aprendida

La solicitud de incorporación de cambios o de fusión es una forma de verificar los cambios en el código con los colegas y comprobar si hay errores, evaluar su preparación antes de pasar a las siguientes etapas y, finalmente, comunicarse con los usuarios finales. También es uno de los principales canales de comunicación entre los desarrolladores. A veces, es posible que ni siquiera conozcamos a una persona fuera de los hilos de las solicitudes de incorporación de cambios.

Muy a menudo, especialmente en las primeras etapas de su carrera, los desarrolladores no prestan atención a su comunicación de relaciones públicas. En concreto, a cómo sus comentarios pueden ser percibidos por otras personas, si los puntos que se expresan son claros y están correctamente escritos en inglés, etc.

Créanme, yo he pasado por eso y también he cometido errores en el pasado. Una experiencia en particular se me ha quedado grabada hasta el día de hoy. La menciono casi siempre durante las discusiones sobre la cultura de las relaciones públicas porque resalta perfectamente la importancia de este tema.

Hace años, cuando buscaba un nuevo trabajo, me presentaron un simulacro de pull request que debía revisar. No pensé en todo a fondo en ese momento porque estaba emocionado por haber recibido una tarea fácil en lugar de otra tortura de Leetcode. Por supuesto, no fue tan fácil. Fue una prueba sobre la cultura de pull request que fallé estrepitosamente.

Leí rápidamente el comunicado de prensa y dejé un montón de comentarios, en su mayoría sin sentido, por ejemplo:

 import UIKit import AVFoundation // It would be nice to list your imports alphabetically, so that it's easier to read.

O otro más:

 func someMethod() { // about 30 lines of code } // What do you think about splitting this method in half?

En otros comentarios poco frecuentes en los que detecté un problema, no fui lo suficientemente claro al respecto y no proporcioné ninguna solución razonable. Pero el principal fracaso de este ejercicio fue que pasé por alto un par de problemas de rendimiento porque mi enfoque no estaba en el lugar correcto. Al final, recibí el siguiente comentario:

Sentí que se perdieron muchos detalles importantes, como la corrección del código, el rendimiento y las mejoras generales (con algunas excepciones menores).

Gran parte de la revisión se centró en el formato, que debería ser un tema de debate en equipo o en el uso de herramientas de formato. Esto también permite que los desarrolladores se centren en los aspectos adecuados de una revisión.

Desde entonces, me di cuenta de que mi enfoque de las revisiones de código estaba lejos de ser perfecto y carecía de muchas cosas importantes.

Este ejemplo es bastante primitivo en cierto modo, pero muestra claramente cómo la falta de una cultura de relaciones públicas puede fácilmente convertirse en un camino peligroso que conduce directamente a errores y problemas en la producción.

Profundicemos en los detalles y comencemos con algo que usted querría evitar como revisor de código: escribir comentarios criticones.

¿Qué es un comentario quisquilloso?

Hay muchas formas de expresar la lógica y los pensamientos en código. Los lenguajes de programación modernos ofrecen la versatilidad suficiente para hacerlo y, a menudo, no existe una solución correcta o incorrecta entre los distintos enfoques. Sin embargo, no queremos que nuestros proyectos parezcan monstruos de Frankenstein: cada equipo debería tener un estilo de código decente con pautas.

Al mismo tiempo, no podemos ni debemos controlar cada línea de código de nuestros compañeros de equipo. Creo que encontrar un buen equilibrio entre ambos es una virtud que idealmente queremos alcanzar.

Un comentario quisquilloso es una solicitud para realizar un cambio, generalmente uno pequeño, que no tiene motivos objetivos para hacerlo. A menudo, es una proyección de una preferencia personal de un revisor de código en el código revisado. Permítanme mostrarles algunos ejemplos:

 // Original guard let newValue, newValue != oldValue else { return } // Change request guard let newValue, newValue != oldValue else { return }

Si me preguntas, siempre elegiría la segunda opción porque, en mi opinión, es más fácil depurar ese código y poner un punto de interrupción justo en la línea de retorno. Pero puedes argumentar que la primera variante ahorra 2 líneas, además, puedes ver este estilo también de los ingenieros de Apple.

 // Original func handleErrorIfNeeded(_ error: SomeError) { if error == .failedRequest { return } if error == .validationFailed { state.errorMessage = “Data input is incorrect” return } // … other cases } // Change request func handleErrorIfNeeded(_ error: SomeError) { guard error != .failedRequest else { return } guard error != .validationFailed { state.errorMessage = “Data input is incorrect” return } // … other cases }

En muchas situaciones lógicas, guard y if en Swift se pueden usar indistintamente. Sin embargo, yo diría que no tienen la misma semántica de lenguaje. guard , como su nombre lo indica, quiere "proteger" el flujo de código de un resultado no deseado, mientras que if es neutral por naturaleza y permite una probabilidad del 50-50% de que se produzcan diferentes rutas de código. Sin embargo, ambas variantes técnicamente no son incorrectas y son perfectamente legibles.

Y el último:

 // Original, the constant value is not being repeated anywhere else func calculateDuration(endDuration: Duration) -> Duration { let startDuration = Duration.milliseconds(130) // … calculation return finalDuration } // Change request func calculateDuration(endDuration: Duration) -> Duration { let startDuration = Duration.milliseconds(Constant.startDuration) // … calculation return finalDuration } enum Constant { static let startDuration = Double(130) }

Se puede discutir sobre la necesidad de extraer cualquier constante en un espacio de nombres separado si se usa solo una vez y cuando no es una constante mágica. En la primera variante, sabes claramente qué es este número de 130. Sin embargo, alguien te dirá que extraigas cualquier constante como esta, sin importar qué.

Todos los ejemplos anteriores son modificaciones menores. Ambas variantes son perfectamente válidas, pueden ser leídas por cualquier persona y no dañan el código. Son simplemente alternativas con el mismo peso en el uso.

Entonces, ¿qué quieres hacer con comentarios como estos? Desde la perspectiva de un revisor, algunos dirán algo como esto:

Bueno, es solo una sugerencia, no es obligatorio aplicarla.

Es cierto, pero al mismo tiempo es importante hacérselo saber al autor, por ejemplo:

Es solo una sugerencia, siéntete libre de descartarla, pero ¿qué opinas de este enfoque: …?

Sin embargo, mi opinión sobre las críticas negativas es que no hay que publicarlas en absoluto . ¿Por qué?

Digamos que enviaste un comentario criticón e incluso destacaste que es una sugerencia de baja prioridad. ¿Qué sucede después?

• 1ª opción: el autor aplica el cambio. ¿Qué significa realmente detrás de escena?
  • Necesitan guardar los cambios actuales que están realizando.
  • A continuación, deben trasladarse a la rama de relaciones públicas.
  • Necesitan aplicar el cambio.
  • Podrían resolver conflictos.
  • Necesitan impulsar el cambio.
  • Luego, se activará una ronda más de pruebas CI debido al nuevo código introducido.
  • Deben responder al comentario con texto o emoji.

Por un lado, es un conjunto muy simple de tareas que hacemos todos los días, como desarrolladores. Pero imagínese, no se trata de un comentario quisquilloso, sino de 10. O tiene 5 solicitudes de incorporación de cambios abiertas al mismo tiempo con quisquillosos de diferentes cantidades. ¿Realmente valió la pena su tiempo y molestia? Cuando sabemos qué es un quisquilloso, la respuesta es no. Prefiero entregar mis cambios más rápido y ahorrar tiempo.

• 2da opción: el autor no aplica el cambio.

  • En muchas situaciones, es de mala educación ignorar el comentario de tu compañero de equipo sin responder en absoluto. Y también puede parecer de mala educación responder simplemente con lo siguiente:

    
    

    Gracias por la sugerencia, pero no me gusta, no la voy a aplicar porque no creo que sea necesaria.

  
  

  • En cambio, como autor, probablemente profundizará en la explicación de los detalles de por qué no aplicará los cambios. Por ejemplo, su visión en guard vs. let como se mencionó anteriormente.

    
    

  • En ese caso, sería fantástico que el revisor simplemente acepte tu opinión y siga adelante. Pero también es posible que sienta la resistencia y responda porque realmente siente que su forma es mejor.

    
    

  • Estos hilos pueden fácilmente salirse de proporción con largos comentarios de ida y vuelta que no son productivos en absoluto. Y al final no van a ninguna parte. Ambos pueden estar de acuerdo en que no están de acuerdo, no hay un "ganador", pero la pregunta final es la misma. ¿Realmente valió la pena el tiempo y las molestias?

  
  

Mi práctica es filtrar esos detalles en las revisiones de código como revisor incluso antes de publicar nada, haciéndome estas preguntas:

¿Es realmente importante cambiar algo?

¿Existen razones objetivas para ello? Si es así, nómbralas.

¿Quiero publicar este comentario sólo porque lo habría hecho de otra manera? ¿O existe una preocupación real?

De esta manera, puede eliminar el “ruido” de los comentarios quisquillosos de su revisión de código y dejar solo las partes importantes, las partes en las que todos necesitan concentrarse en la escalabilidad, la robustez, la seguridad de los subprocesos, etc.

Pero, ¿por qué aparecen estos comentarios? Por lo general, es una señal de que los revisores no tienen una idea clara de en qué deben centrarse o qué buscar en las solicitudes de incorporación de cambios. En la siguiente sección, analizaremos exactamente eso.

Cómo crear una cultura de revisión de código

Veamos cómo podemos crear un modelo de un “estándar de oro” de relaciones públicas. Enumeraré un conjunto de valores y pautas que, en mi opinión, son esenciales para mejorar las revisiones de código:

1. Para todos: defina un conjunto de categorías para sus comentarios de revisión de código y asígneles distintos niveles de gravedad. Muestre siempre ejemplos a sus colegas, para que todos estén en la misma sintonía. Ejemplo:

…Y así sucesivamente. Puede ser diferente en tu equipo, pero la idea aquí es tener una estructura y una clasificación. Incorporar valores de gravedad nos permite priorizar mejor y centrarnos en lo esencial.

2. Revisor : intente asignar un espacio de tiempo específico para las revisiones de su código. No se apresure y permítase profundizar en el significado de los cambios de código, no en la sintaxis. Esto debería mejorar la calidad y la claridad de sus comentarios.

   
   

3. Autor: Ayuda a tus compañeros. Si sabes que hay algunas partes que pueden no estar claras debido a la falta de contexto o porque tu solicitud de incorporación de cambios es solo una parte de una serie de cosas que vendrán, simplemente deja un comentario con una explicación sobre tu propia solicitud de incorporación de cambios con anticipación. ¡Ahorrarás tiempo para ambas partes!

   
   

4. Revisor: vuelva a leer sus comentarios antes de publicarlos. Póngase en el lugar de la otra parte. ¿Está todo claro? Intente ceñirse a la siguiente estructura:

   1. Exprese la preocupación que tiene.
   2. Por qué es importante
   3. ¿Qué sugieres hacer en su lugar?

   
   

5. Para todos: revisen su gramática y ortografía en inglés. Sí, para muchos de nosotros, el inglés no es nuestra lengua materna y, a veces, es algo difícil de hacer. Pero si no está bien escrito, puede dar lugar a todo tipo de confusiones y errores que no queremos tener.

   
   

6. Para todos: ¿Cómo se percibe tu comentario? ¿Suena amistoso? Este punto es bastante similar al anterior, pero la idea es mantener un enfoque saludable y amistoso en la forma escrita. Por ejemplo:

   
   

   Debes extraer esta lógica en un método separado.

   
   

   Aunque no hay nada gramaticalmente incorrecto en esta oración, la palabra “must” rara vez se usa en este contexto y puede considerarse como una orden para el lector. Los funcionarios del gobierno te dirán lo que debes hacer porque existen leyes, pero no es el tipo de redacción que quieres usar con tus colegas cuando estás en el mismo equipo. Prueba esto en su lugar:

   
   

   ¿Qué opinas de extraer esta lógica en un método separado?

   
   

7. Revisor: Equilibre lo negativo con lo positivo. Si dejó algunos comentarios críticos, busque algunos buenos y resáltelos también. Es un detalle pequeño, pero mejora la percepción general a los ojos del autor.

   
   

8. Revisor: si todo está bien en los cambios de código, no dudes en publicar elogios. Las aprobaciones de Instagram sin ningún cartel no son un buen comportamiento y dejan mucho que cuestionar. Publica un comentario con las cosas exactas que consideraste excelentes en el trabajo de tu colega. El simple "LGTM" es demasiado genérico y puede percibirse como despectivo.

Creo que estos son buenos puntos para empezar en tu equipo y trabajar hacia una comunicación mejor y más saludable en tus solicitudes de incorporación de cambios. La parte más difícil después de eso es seguir estas pautas. A veces, puede ser dos pasos hacia adelante y un paso hacia atrás.

Conclusión

Al final, la codificación no siempre es la parte más difícil. La comunicación y la colaboración pueden ser más desafiantes, especialmente cuando todos provienen de diferentes países y orígenes con sus propias experiencias y pensamientos.

Espero que esta perspectiva sobre la cultura de revisión de código te haya resultado útil y tal vez te resulte útil.

¡Buena suerte y revisiones de código amistosas para todos ustedes!