En dev&del estamos en proceso de introducir el uso de los pull request, y aprovechando que el Pisuerga pasa por Valladolid implantamos Code Review, la cual es una de las habilidades más simples de introducir en el ciclo de desarrollo, y bien hecho es una forma maravillosa de trasladar conocimiento entre compañeros y mejorar la calidad del código del proyecto.

Aunque el nombre es intuitivo, cada uno podría interpretar libremente en qué se debe fijar a la hora de revisar, por tanto vamos a definir qué vamos a entender por code review y cómo lo vamos a llevar a cabo en dev&del.

¿Qué es Code Review?

Es un proceso en el cual solicitas a uno o más compañeros que revisen tu código. Esta solicitud se puede llevar a cabo en cualquier momento de la vida de la feature que estás desarrollando, pero de momento vamos a focalizarlo en el momento del pull request.

Al menos dos desarrolladores del equipo revisarán los cambios que se van a introducir en la rama principal, y, anotarán comentarios en los bloques de código que consideren necesario revisar. En dichos comentarios podrá haber notas acerca de la calidad, complejidad, diseño de la solución, felicitaciones por la calidad entre otras opciones.

La idea de este proceso es tanto mejorar el código que vamos a entregar a nuestros clientes como aprender de nuestros compañeros, ya sea con los comentarios a nuestro código, como leyendo el código escrito por otros y ver qué solución han adoptado.

Objetivos del Code Review

En dev&del implantaremos Code Review persiguiendo 2 objetivos fundamentales:

Calidad del código

El software, en estado salvaje, se va pudriendo con el tiempo. Sin la adecuada atención y cuidado, la complejidad de los métodos va aumentando, corregir incidencias añadiendo un if aquí y allá, desarrollar nuevas funcionalidades haciendo un “copy-paste” de otra parte del código generando duplicidades entre otras prácticas a corto plazo puede ser la opción más fácil y rápida, pero a la larga nunca es la opción correcta. Los revisores del código actuarán como guardianes de la calidad, persiguiendo que cada pull request no empeore la calidad del proyecto, idealmente que la mejore, serán esa barrera contra la pereza que está innata en todos nosotros.

Mentoring

El mundo del desarrollo es un mundo muy amplio, y es probable que todos en esta empresa podamos aprender algo de los demás. Que revisen nuestro código es una muy buena forma de hacerlo. Temas como aprovechar las funcionalidades que nos ofrece el framework, mejores formas de aproximarse a una solución o algo tan básico como nombres más intuitivos para nuestros métodos y variables son pequeñas píldoras de conocimiento que nos daremos unos a otros durante el Code Review que no sólo mejorarán la calidad del código desarrollado, si no que mejorará nuestra calidad como desarrolladores.

¿Cómo revisar el código?

En primer lugar, abre el pull request y échale un vistazo rápido a toda la solución. A continuación, míralo más en detalle, fijándote en los siguientes aspectos.

Diseño

Uno de los principales elementos en los que nos fijaremos a la hora de revisar el código es que el diseño general del cambio, el cómo interactúan los elementos entre sí, la estructura de los paquetes empleada, se mantiene el respeto por el principio de responsabilidad única, etc.

Funcionalidad

¿Hace el cambio lo que se supone que pretende hacer? Esto no implica arrancar el proyecto y probar la aplicación (somos reviewers, no testers funcionales), este punto tiene más que ver con el impacto que podría tener en el método o la funcionalidad modificados (evitar consecuencias no deseadas en otras partes del código). Pueden ser difíciles de detectar o pasarse por alto, pero cuatro ojos ven más que dos.

Complejidad y legibilidad

Revisar que el código implementado sea legible. Idealmente sería saber exactamente qué hace cada método únicamente leyendo su nombre, para lo cual, necesitaría un nombre explicativo, y que el código que contuviese se limitase a hacer lo que dice su nombre.

Al modificar un método complejo es mucho más probable que introduzcamos un bug sin darnos cuenta.

En cuanto a los comentarios introducidos en el código, habrá que fijarse en que la intención del comentario sea explicar por qué un método hace lo que hace, no el qué hace (esto lo debe hacer el propio código).

Tests

Todo el código que suba, debe tener una cobertura del 100% de tests unitarios (si excluimos modelos y controladores), y una cantidad razonable de tests de integración y E2E, pero ya entraremos en ello.

Buscamos mejora, no perfección

Todo lo anterior describe un ideal, pero tenemos que ser pragmáticos y debemos mantener un balance entre calidad y tiempo. De momento, el objetivo inicial de los reviewers consistirá en que la calidad general del proyecto mejore. Si la estructura de la solución es correcta, no se ha introducido más caos, ni se han generado duplicidades de código, aunque haya mejoras posibles (cambiar un sistema de herencia por una inyección, cambiar alguna funcionalidad por AOP o alguna otra sugerencia) no se parará el pull request, aunque estas mejoras se podrían comentar como un nit para conocimiento para el futuro del desarrollador.

Nit (Notas a tener en cuenta): En empresas con gran calidad en su software, que tienen interiorizado el uso de los patrones de software, del testing, de los principios SOLID, del code review… los comentarios nit se usan para indicar aspectos no importantes que no van a bloquear un pull request como puede ser un typo en un comentario o un doble ; erróneo.

Escritura de los comentarios

“Elimina esto”.

“Este código no se entiende”

“Renombra el método a updateCaseStatus()”

“Esto está mal, el caso XXXX no está contemplado”

“Te has olvidado de incluir el close del stream en el finally”

Lo que vemos aquí son comentarios que, aunque pueden ser correctos y estar evitando que un bug se integre en la rama principal, no están expresados de la mejor forma y pueden dirigir hacia un mal ambiente laboral. Tenemos que tener en cuenta que los desarrolladores somos personas y nunca sienta bien que te señalen tus errores, por lo que debemos darle el cuidado que se merece a la escritura de los comentarios, y tener en cuenta que el code review es una comunicación no verbal, por lo que la misma frase puede ser leída con distintas entonaciones, dando lugar a asperezas entre revisor y revisado.

Algunos tips a la hora de añadir un comentario.

Ok, mi código ha sido revisado, ¿y ahora qué?

Lectura de los comentarios

A la hora de leer los comentarios que han hecho sobre tu código, ten en cuenta lo siguiente:

Realización de las mejoras propuestas

Lleva a cabo las mejoras que te hayan propuesto. Contesta a todos los comentarios para que ambos estéis al tanto de lo que se cambia y nada se pase por alto. Ante la duda júntate con el Reviewer.

Resolución de conflictos

Puede haber un punto en el que revisor y revisado no se ponen de acuerdo sobre el mejor enfoque de la solución. No dudéis en incluir a gente de otro proyecto para que dé su punto de vista.

Referencias

Aquí os dejamos referencias de gente muy lista hablando sobre el tema: