Jesús L.C. – Apuntes de un aprendiz

¿Todo tiene que ir en code review? ¿Hay que hacer la code review antes?¿por qué no la hacemos después de mergear? Si ya hacemos pair/mob programming ¿tiene sentido también una code review? Ya hemos hablado antes de como hacer code reviews con fundamento, de que las code reviews son algo más que código: son comunicación entre personas. También hemos visto cómo llevar a cabo una code review y qué puntos tener en cuenta. Las code reviews son un «estandar de mercado» todo el mundo las hace porque X. Dependiendo de nuestro contexto, de cada equipo, de cada empresa,… puede que haya otras prácticas ténicas que mejoren nuestra productividad y el flujo de valor hacia el usuario.

Partamos de la base de que esto que pongo aquí es mi opinión este post se basa en mi «cámara de eco» ( experiencia trabajando, opiniones compartidas, libros, artículos y recursos que he ido viendo,…) La idea de este post es que reflexionemos sobre las prácticas que tenemos en nuestro equipo y de que vez en cuando nos preguntémos el porqué de esas prácticas que utilizamos y si siguen teniendo sentido en el contexto actual.

¿Para qué sirve una code review?

Hablemos de que es lo que aporta una code review. En la industria, las code review tienen 2 objetivos principales: compartir conocimiento y mejorar la calidad del código.

Una code review aporta puntos de vista distintos a la hora de resolver un problema o de como se ha resuelto un problema. Es cierto que las code reviews nos pueden ayudar: nos proporcionan feedback, nos enseñan distintos usos de una función o una forma diferente (a la que teníamos en la cabeza) de resolver un problema. Pero una cosa es que veamos como revisores una arrow function y pensemos «pues no se me había ocurrid resolver esto así» y otra muy distinta es que la próxima vez que nos encontremos con un problema similar usemos un arrow function. Por lo tanto, debemos reflexionar si el tiempo que estamos invirtiendo en las code review de verdad nos está aportanto «conocimiento compartido» o si quizás haya otras maneras de compartir conocimiento.

Del mismo modo, cuando hablamos de mejorar la calidad del código. ¿Estamos sacando partido a las code reviews? Hay muchas herramientas de análisis estático como Psaml, PHPStan, Sonarqube,… o incluso para chequear deptrac. Tambien existen técnicas para mejorar nuestro código como aplicar testing, mutant testing, hacer TDD, o maneras de mejorar el flujo de trabajo como automatizar el flujo de subida a producción,… Entonces preguntémosnos ¿para que sirve una code review? ¿Es necesaria una code review antes de subir a producción? ¿De verdad le estamos sacando partido al tiempo que invertimos en una code review? Ya sea como revisores o como creadores de esa code review todo el tiempo que utilizamos nos está sirviendo para mejorar o podríamos invertir ese tiempo de otra manera. Vayamos por partes, un pro de las code reviews es que nos ayudan a homogneizar el código.

Homogeneizar el código

Como hemos comentado antes, uno de los argumentos a favor de hacer code reviews es que es una buena herramienta para homogeneizar el código, para que todo el equipo escriba código de la misma manera. Si bien es cierto, una code review si puede servirnos para compartir una idea de cómo realizar un cambio, o para compartir un refactor o incluso para recibir feedback de si la solución que optamos va por buen camino. Como comentamos al principio, la idea del post es que nos preguntemos si las code review nos están aportando valor como equipo y si les estamos dando la importancia que tienen (o usamos el tiempo de la siesta después de comer para quitar las code review que me han asignado.

Las code review mejorarán la calidad del código, pero… ¿Podemos ir una paso más allá? ¿Qué podemos añadir/modificar en nuestro flujo de trabajo para minimizar la carga cognitiva de una code review? ¿Como podemos hacer que las code review sean más fluidas?

Linter:

Tener herramientas de análisis estático en nuestro pipeline es esencial: Psaml, PHPStan, Sonarqube, o incluso podemos revisar la arquitectura con deptrac. Y no solo tener estas herramientas, sino el proceso automatizado para ni siquiera permitir una code review si no se cumplen una serie de caracteristicas en nuestro pipeline.

Tener tests:

Incluir test hace que podamos entender fácilmente mejora el código y reduce el trabajo del revisor. En la code review, como revisores podemos comprobar que tenemos test suficientes y que hemos cubierto todos los posibles caminos, incluso usar mutant testing para chequear que nuestros test/código son robustos. También si hemos elegido una «manera» de escribir los test, por ejemplo seguir el patrón Given/When/Then, o si hemos elegido una nomenclatura para las funciones puede ser un buen momento para chequearla.

Mantener una lista de errores comunes:

Si tenemos una lista con los errores que hemos cometido en el pasado, podemos evitarlos en el futuro, así el revisor no tiene que invertir tiempo en hacer una explicación detallada, sino simplemente enlazar el documento. Por ejemplo aquí tenemos la de Go https://github.com/golang/go/wiki/CodeReviewComments

Descripción de la code review

Cuando somos los creadores de una pull request es importante que esta tenga una buena descripción. Tenemos que ponernos en la piel de la persona que revisa y proporcionar todo el contexto que podamos. Para el revisor tendría que ser sencillo entender de un vistazo todo el contexto antes de entrar en el código. Por ello una pull request debería incluir cosas cual es el objetivo: ¿por qué de este cambio?, detallar los diferentes cambios de la pull request y los test que se han hecho, Un enlace a la User Story para tener más información, describir la performance y la seguridad, añadir un esquema, la relación de las tablas,… tener una plantilla para todo esto hace que como creadores no nos aparezca la pereza a la hora de crear descripciones y ayuda a que no nos olvidemos de ningun punto importante. Añadir imágenes para explicar la PR o incluso un pequeño video (Loom es de las mejores extensiones que he visto para eso) aporta muchísimo, no solo a la persona que revisa sino a nuestro yo del futuro. Si hay un bug en el futuro será más sencillo ver que cambios se hicieron y entender todo el por qué de los mismos.

Por tanto, si nuestro objetivo es homogeneizar al código y hemos tenemos unas buenas herramientas automáticas para chequear todo lo posible,… la parte de poner en común con el equipo ¿podemos hacerla después? Si creemos que el código tiene la suficiente calidad como para ir a producción,es decir, si como persona que ha desarrollado el código, estamos seguros que esa funcionalidad es lo suficientemente buena como para que sea candidata para ir a producción… y la idea es usar las Conde review como proceso para homogeneizar ¿se podría hacer un cambio posterior? Tener una Code review esperando porque «se podría usar X como alternativa» o “el nombre de esa función no es bueno,… quizás sea más eficiente crear una reunión de «code review» donde entre todo el equipo se comenten algunas piezas de código con la idea de homogeniezar, de buscar nombres comunes o de evitar leaks o de aprender como se ha solucionado un tema de performance. A mi me encanta una charla de Xavi Gost en la que habla de “Desarrollo dirigido por Consenso” creo que puede ayudarnos a tener otro punto de vista en la manera de compartir conocimiento.

Encontrar errores en el código

Otro de los factores que se manejan en la industria a la hora de hacer code reviews es que son útiles para encontrar errores. A mi me resulta muy comlicado encontrar errores en una code review, si tiene unos cuantos ficheros (4 o 5) quizá si pero con 20 ficheros encontrar algo no es una tarea sencilla. Sobre todo si lo estamos haciendo en el navegador web sin ejecutar el código. ¿Seguro que es la mejor manera de encontrar un bug? Para encontrar bugs hay otras herramientas como pueden ser testing, ya se exploratorio, o mutant testing , chaos testing…

Quizás si se puedan hacer comentarios sobre code smells, pero al final es un no deja de ser un comentario donde como revisores quizás no estemos teniendo en cuenta todo el contexto. De cualquier forma, vamos a parar todo el flujo de valor hacia el usuario porque ¿hay una probabilidad remota de encontrar un bug?

En el caso de que nuestro argumento para hacer code reviews en un equipo sea la posiblidad de encontrar un bug a lo mejor podriamos invertir ese tiempo en: hacer testing exploratorio, mejorar la cobertura de test, mejorar los linters, mejorar la automatización, tener un proceso de despliegue robusto y confiable, hacer un paso de rollback, usar feature toggles, canary releases,… en fin unas cuantas técnicas que darían para otro post.

Compartir conocimiento

Otro de los puntos a favor de las code review es que nos permiten compartir conocimiento. como comentamos es cierto que una code review puede servirnos para compartir una idea de cómo realizar un cambio, o para compartir un refactor o incluso para recibir feedback de si la solución que optamos va por buen camino. Pero creo que el hecho de leer la solución de otra persona, al menos a mi, no me hace interiorizar esa solución para usarla en el futuro. Quizás hacer algunas sesiones de parir programing para partes complicadas: refactor complejos, o aplicar un patrón o entender alguna parte del código pueden aportarnos más.

En un contexto de equipo que trabaja de manera síncrona, en el mismo huso horario, deberíamos preguntarnos ¿por qué un incremento de software que podría estar en producción aportando valor, este parado porque es necesaria una code review? Estamos cortando el flujo de entrega de valor ¿sólo para asegurarnos de que hemos puesto en común los nombres de las variables? Si no estamos seguros de como afrontar parte del problema, quizás estaría bien que pidiésemos ayuda para que otra persona del equipo nos eche un cable (hacer de pato).

Del mismo modo, siendo proactivos, escribiendo documentación, haciendo esquemas, diagramas de cómo funciona una parte del código, ofreciendo ayuda y pidiendola podemos hacer que el flujo de valor sea continuo y por ello ayudar a que el conocimiento esté en todo el equipo y no solo en las cabezas de algunas personas.

Elementos comunes en las code reviews

Las code reviews son el estandar del desarrollo software hoy en día. Ya hemos comentado para que sirve, ahora vamos a revisar algunos elementos comunes que podemos encontrarnos utilizando esta herramienta. Si nuestra idea es utilizar las code review en un equipo, lo mejor es que podamos sacarle el máximo jugo. Por ello hay ciertos patrones que se repiten y que debemos tener en cuenta:

Número de cambios en una code review

¿Cuantos cambia debería tener una code review? La idea es que el tamaño se óptimo para que ese cambio en el código aporte valor, pero sin que sea una pesadilla para la persona que revisa. Ya sea porque el cambio es muy pequeño (renombrar una función privada de una clase) o demasiado grande (cambiar la arquitectura de un módulo).

Si una code review es grande (más de 20 ficheros) es muy difícil localizar un bug, es complicado conocer el contexto de todos esos cambios y lo peor de todo que seguro que va a tardar más en que sea revisada.

Si la code review es pequeña, lo mismo encadenamos un pipeline de code reviews porque un cambio que hicimos en la primera es necesario para un cambio posterior, lo que lleva a un lío de code reviews abiertas donde podríamos perder el foco en el producto.

¿Cual es el tamaño perfecto? Dependerá del equipo, de la naturaleza del cambio, de lo automatizada que este el flujo de trabajo…

Refactoring en code reviews

Es de las mejores code reviews, cuando hacemos una mejora en el código para que nadie mas del equipo tenga que perder tiempo en entender esa parte del código. Para mi el mayor problema es que los refactor me los encuentro en mitad del desarrollo de una feature, por lo que tengo que elegir entre no hacerlo, hacerlo luego en otra rama, hacerlo en esta mezclado con lo que estoy haciendo,…

El problema de esto que haciendo code review, estamos perdiendo la oportunidad de hacer refactoring oportunista ya que si encontramos un trozo de código a mejorar nos pensaremos mucho el cambiarlo porque eso aumentará el numero de ficheros modificados y por ello la posibilidad de conflictos (no solo de código) sino de fricciones con el equipo por las code review tan grandes.

Que revisar en las code reviews

Pues depende del equipo, la madurez, como sea el flujo de trabajo,… de eso ya hablamos hace tiempo en el post que puntos a tener en cuenta en una code review. Lo que sí debemos ser conscientes es que si algo se envía a una code review es porque es susceptible de subir a producción y por ello debe tener la calidad suficiente para ello. En la imagen de abajo se ve a un desarrollador diciendo algo así como: no hace falta chequearlo de nuevo, ya lo verá la persona que revisa. En la otra viñeta se ve a la persona que revisa diciendo que puede como eso ya lo han mandando a codera review seguro que está bien y no necesita “diu le check”

Prioridad de la code review

Si las code reviews son tan importantes: ¿por qué las hacemos después de comer o en cuánto tenga un hueco libre? Si todo el código debe de pasar por code review, estas deberían ser una prioridad para todo el equipo: tanto para el que ha escrito el código p.e. buscando revisores, como para el resto del equipo. Sino lo unico que hacemos es parar el flujo de valor porque «es necesaria» una code review.

El problema o excusa habitual que venimos escuchando con dar prioridad a las code reviews es el «cambio de contexto» que suponen tanto para la autora como para las revisoras.

Por un lado la creadora, crea la code review, añade revisoras y se va a coger una tarea nueva del backlog. Cuando llegan comentarios de la code review tiene que dejar lo que está haciendo o buscar un hueco para cambiar de contexto y contestar los comentarios o realizar algún cambio. Si tiene 2 o 3 code reviews abiertas eso puede suponer un esfuerzo. Lo mismo sucede en las revisoras, te llega una notificación de que te han asignado a una code review, buscas un hueco para leer las descripciones y entender el código y hacer apreciaciones con fundamento. Para cualquier persona del equipo que este de revisoras y de creadora eso supone cambiar de contexto cada poco.

Hay equipos que usan pomodoros para sincronizarse, así que si todo el mundo esta en el mismo pomodoromomen el descanso hacer code reviews de manera sincrona o al menos en una llamada hace que sea sencillo compartir feedback. Eso si, debemos tener en cuenta que cada persona piensa de una forma distinta, su ritmo distinto y compartir pantalla para revisar un cambio podría no ser lo mejor. Quizás hacer parir programing o similar podría funcionar en otros casos, depende del contexto.

comentarios cuantos y como

Si una code review tiene muchos comentarios puede ser un smell de que quizás haya que sentarse juntas y ver como plantear el problema de nuevo. Esto puede deberse a diversos factores: no se ha entendido el contexto (ya sea la revisora o la creadora) no se han tenido en cuenta todos los factores,… por ello poder compartir de manera sincronizada mediante una llamada código y desarrollar con tests es un buen paso. Del mismo modo ser asertivos a la hora de realizar comentarios y tener siempre en mente la idea de ayudar. Al fin y al cabo todas queremos incrementar el valor que estamos aportando a los usuarios, por lo que llegar a un punto de equilibrio va a ser sencillo.

Para mi, una de las herramientas sube mas me aporta a la hora eliminar fricción y de evitar algunos de estos bloqueos es desarrollar con «feature flags». La idea de las “feature flags” es desplegar el código sin miedo a que un usuario lo vea y se sienta afectado por el mismo. Un feature flag no es más que un «if» que se ejecuta bajo ciertas condiciones. Sabiendo esto podríamos desplegar nuestro código sin necesidad de una code review antes. Por ello hacer “slicing” de las funcionalidades pensando que se puede subir y activar solo para algunos usuarios hace que las code reviews sean menos “de vida o muerte” porque podemos desplegar sin miedo a que algo no funcione y por tanto disminuir la presión en la code review, sobre todo si tenemos gran parte de nuestro flujo de trabajo automatizado.

Conclusiones

Al final ha quedado un post bastante extenso, la idea del post es que reflexionemos si las prácticas que usamos son las mejores en nuestro contexto. Si hay algo que podemos mejorar o probar para ir un paso más allá. El hecho de que las code review se hayan convertido en estándar no significa que tengamos que usarlas siempre o que tengamos que poner todo el peso/carga sobre ellos.

Hemos visto herramientas, consejos y distintas maneras de afrontar una situación como son las code reviews. El argumento de homogeneizar el código puede que no sea lo suficientemente robusto o que tengamos a nuestra disposición otras prácticas para ese propósito. Del mismo modo, encontrar un bug en una code review es algo complicado y quizás podríamos utilizar ese tiempo en formación, parir programing o reuniones de consenso.

Si la code review son para compartir conocimiento o mejorar la calidad ¿Se pueden hacer que todos los cambios que hagamos en el código sean para mejorar la calidad o para aporta nuevos conocimientos que compartir? Quizás adoptar un framework como Ship / Show / Ask pueda ayudarnos más, pero la explicación del framework la dejaremos para otro post.