Revisar el código de otros compañeros antes de una pull request o antes de pushear es una práctica bastante extendida. Pero ¿cómo hacerlo de manera efectiva? Hace tiempo escribí un post sobre «Cómo llevar a cabo una code review y qué puntos tener en cuenta « pero después de la «Software Crafter Madrid» me puse a buscar información sobre como mejorar la comunicacióne como intentar disminuir la fricción en las code reviews.
La idea de este post no es escribir en piedra una serie de normas, sino comentar una serie de trucos que a mi han funcionado y por eso me gustaría que tú te animaras a contarme los tuyos. Abajo dejo una serie de referencias
¿Qué es una code review?
Para entrar en contexto, con code review me refiero a leer e inspeccionar el código de otros compañeros de equipo, ya sea en persona o a través de una herramienta.
Así pues, tenemos a una persona autora que realiza una serie de cambios en un código fuente con un objetivo (refactorizar, crear una feature,…) y por otro lado tenemos a uno o varias personas revisoras que son las encargados de inspeccionar esos cambios.
Cuando las personas que revisan terminan pueden aprobar esos cambios, rechazar los cambios y/o hacer comentarios con respecto a aspectos del código, del objetivo del cambio,…
¿Por qué son complicadas a veces las code reviews?
Porque tu como persona que programa envías una serie de cambios, que te han supuesto un esfuerzo, donde has invertido tiempo, además crees que esa code review va a servir para compartir conocimiento. En definitiva estas aportando y esperas recibir el Ok. Así que cuando recibes comentarios, estas recibiendo feedback.
Por ello como persona que programa tienes que estar abierto a cambios, a aceptar criticas, a tener una actitud de aprendizaje y mejora.
Bajo mi punto de vista dar ese feedback es complicado, porque la situación es delicada. Como revisores debemos medir muy bien que es lo que decimos y sobre todo como lo decimos. Además si las code reviews son leidas (online, con una herramienta,…) perdemos el lenguaje corporal, por lo que aún tenemos que tener más encuenta esos comentarios.
Por ejemplo un comentario como «te has olvidado cerrar el handler del fichero», podría ser interpretado «No puedo creer que seas tan idiota como para olvidarte cerrar el handler».
Otro ejemplo, cuando revisas el código de alguien y ves que una de las funciones no está bien, ha usado un hack muy feo o cualquier cosa ¿como lo comentas?
- Respuesta A:: ¡Vaya mierda! ¡Quita ese hack! No uses un switch con 10 cases.
- Respuesta B: En está otra función hiciste una buena abstracción, no te parece que en esta podríamos segur la misma filosofía.
Normalmente, cuando tenemos confianza tendemos a tirar por la respuesta A, yo incluido, pero pensándolo fríamente la 2 respuesta aporta mucho más feedback. La persona que programa puede haber hecho ese hack porque no sabía que hay otra manera de solucionarlo, porque no se dio cuenta, o porque ese hack ya es´ta en otra parte del código y nadie dijo nada, con la respuesta B creo que estamos dando más feedback.
Algunos trucos para las code reviews
Automatiza todo al máximo
Las conversaciones en las code reviews tendrían que estar relacionadas con la simplicidad, los nombres de variables, la documentación generada,… no sobre espacios/tabuladores, lineas en blanco,…
Un comentario de tipo: usa 4 espacios, las llaves van en la siguiente linea… no aportan y aun peor, no deberían haber llegado a la revisión cosas como esa
Lo ideal sería:
- Elegir un code style
- Tener hooks de git para que no deje commitear si no se cumplen las reglas
- Incluso añadir reglas custom para ciertas cosas propias del equipo: mensajes de commit, indentación,…
- Automatizar con jenkins o similar:
- Que el estilo de código es correcto.
- Que los test siguen en verde.
- Que la cobertura no disminuye.
Las reviews tienen prioridad
Tendríamos que tratar a las reviews como hechos importantes y darles prioridad. Idealmente en el primer hueco disponible.
Si un compañero te envía una review, significa que ha terminado y que está bloqueado. Podría empezar otra tarea, pero cuando llegasen los comentarios de la review, tendría que dejar los cambios que tiene (stash) y ponerse con los de la review.
Dando prioridad, ayudas a que la persona que programa haga tareas más pequeñas porque sabe que así no va a bloquear con tanta facilidad. No digo que tengas que dejarlo todo por hacer la review, pero si hacer que el ciclo sea más rápido y no tengan que recordartelo.
Poner ejemplos en los comentarios
Cambia ese nombre de variable, ese método tiene un nombre horrible,… son comentarios que no aportan valor. Tú como revisor si tienes un nombre alternativo deberías proponerlo y si no se te ocurre ninguno mejor es que quizás ese sea el correcto. Puede que la persona que programó ese día no estuviese inspirada, no conozca toda la terminología de negocio o simplemente que se le de mal poner nombres.
Así que es bueno poner ejemplos, pero no solo en los nombres de variables o funciones, sino en todo lo que se te ocurra. Si hay una abstracción que no termina de encajar en la review que estás haciendo tú como persona que revisa da un ejemplo de algo que sí esté bien.
No dar opiniones
Al hilo el anterior, sería interesante no dar opiniones en los comentarios, sino escribir comentarios constructivos e incluso dar alguna referencia.
Por ejemplo: Esta clase se encarga de descargar y parsear el fichero. ¿Podríamos intentar mejorarla basándonos en SOLID y hacer una clase CurlDonwloader y otra PDFParser?
Del mismo modo meter algún link o incluso si ves que hay una parte del código que no se termina de entender proponer hacer una pequeña reunión, una sesión sobre patrones de diseño o lo que sea.
Intentar no usar tú
Puede parecer una tontería, pero volvemos al principio. La persona que programó, hizo un esfuerzo y no queremos crear mal ambiente ni que se ponga a la defensiva porque se sienta atacada. Por eso creo que es mejor utilizar el nosotros.
Por ejemplo:
- con tú: ¿puedes poner un nombre mejor, algo como productAvailable?
- con nosotros: ¿podemos renombrar esto a algo como productAvailable para que sea más descriptivo?
El nosotros refuerza eso de la propiedad colectiva del código.
Cuidado con dar mucha caña
Si en una review haces 25 comentarios, puede que la persona que programa se sienta abrumada por tantos comentarios. Esto no significa que bajemos el nivel de calidad, sino que puede ser un síntoma de que la persona que programa no conoce bien todavía el negocio, la base de código o que se podría haber usado un patrón u otro.
Como hemos comentado, las reviews son para compartir conocimiento en todas direcciones. Por ello quizás pueda ser bueno ir a ver a ese desarrollador y/o brindarle nuestra ayuda.
Otra opción podría ser en un primer momento solo comentar sobre temas estructurales o de patrones, para en una segunda revisión ver los métodos, nombres y demás. Aunque cuidado que si la revisión da muchas vueltas también puede ser frustrante para ambos.
No dar ordenes sino preguntar
En mi opinión es mejor expresas los comentarios como una pregunta para ayudar al dialogo. Se trata de que aprendamos entre todos y sobre todo ten en cuenta que en la review solo ves el principio y la solución del código. No se ve el proceso que se ha seguido hasta llegar a esa solución, por lo se pierde un poco de contexto.
Por ejemplo:
- orden: Mueve estas clases a una carpeta aparte
- pregunta: ¿crees que podríamos mover esto a una nueva carpeta para tener separadas estas responsabilidades?
¿Crees que queda mejor el segundo comentario?
Dar feedback positivo
No solo tienes que comentar lo que está crees que puede mejorar, es bueno realizar comentarios positivos. Es lo de siempre, hay un esfuerzo puesto en juego, Por eso agradecer sinceramente esa refactorización, esa clase cortada en 2, esa abstracción que te ha sorprendido ayuda a segur adelante.
No ser pedante
No hay que seguir todos los consejos al pie de la letra, porque al final son solo eso consejos y trucos que pueden ser útiles en algunas ocasiones. Somos profesionales, intentamos mejorar cada día y tenemos confianza hacia nuestros compañeros de trabajo. Así que creo que cuando ya hay un rodaje no hace falta escribir 35 lineas de comentarios justificandose. Pero siempre tener en cuenta a la otra persona.
Conclusiones
Al final estos son solo pequeños trucos sobre como reducir la fricción en el ciclo de desarrollo. Todos tenemos un objetivo común: entregar valor y mejorar el producto. Por eso bajo mi punto de vista, si cuidamos un poco el lenguaje y nos ponemos en la piel de la otra personal antes de enviar el comentario, seguramente esa entrega de valor sea más efectiva.
A modo de pequeño resumen
- Automatiza todo al máximo
- Las reviews tienen prioridad
- Poner ejemplos en los comentarios
- No dar opiniones
- Intentar no usar tú
- Cuidado con dar mucha caña
- No dar ordenes sino preguntar
- Da feedback positivo
- No ser pedante
Y como he dicho antes todo esto puede resumirse en: cuidar el lenguaje y ponerse en la piel del otro, algo muy relacionado con la asertividad.
¿Y cuales son tus consejos a la hora de hacer reviews? ¿como haces las reviews?¿qué es lo que lees?
Jesús L.C. - Apuntes de un aprendiz 
Comenta la entrada