Jesús Ramírez Guerrero

¿Qué es un Pull Requests?

Es una gran manera de construir software en equipos – en particular para los equipos distribuidos; no sólo para el desarrollo de código abierto, sino también en las empresas. Inicialmente comenzaron con proyectos de software libre, al ser estos de naturaleza puramente distribuida. Se trata de una solicitud de aprobación de código dentro de un equipo, para ello un número concreto de desarrolladores / testers / arquitectos proporcionarán mejoras, darán visto bueno y comentarán trozos de código de cierta tarea.
Tras leer mucho leer y debatir en foros especializados, he ido recopilando varios puntos que, a mi juicio, considero claves a la hora de entrar y salir en la espiral que pueden llegar a ser los odiados-amados Pull Requests

1. El tamaño sí importa, cuanto más pequeño mejor.

Un Pull Request pequeño tiene mucha más facilidad de que sea aceptado. Lo primero que hago cuando recibo una notificación acerca de un Pull Request es revisarlo para tener una idea acerca de su tamaño. Se necesita tiempo para revisar adecuadamente uno, y en mi experiencia, el tiempo que tarda es exponencial con el tamaño; la relación ciertamente no es lineal.

En el caso de tener entre manos un gran Pull Request, lo primero que deberemos hacer es mirar el tamaño y complejidad del mismo, puesto que deberemos organizarnos para darle la importancia que se merece. De nada sirve revisar en 5 minutos o de pasada un trabajo realizado en días, semanas o incluso meses. Opcionalmente incluso podriamos declinarlo y pedir desglosar el Pull Request en otros más sencillos, aunque esto dependa de la subdivisión de la tarea y la gestión de la responsabilidad interna en un equipo…

¿Cual es el tamaño adecuado? Obviamente, depende de la finalidad, tarea y proyecto pero en general, menos de una docena de archivos está dentro de lo racional.

2. Centrarse en una sola tarea.

Al igual que el principio de la Responsabilidad Individual en el que se establece que una clase debe tener sólo una responsabilidad, en un Pull Request se establece que no debería haber más responsabilidad que la de añadir al proyecto de una funcionalidad acotada y determinada previamente (y diría que establecida en un organizador de tareas), pero nunca más de eso.

El por qué de hacerlo así es porque, si agrupamos tres tareas en un mismo Pull Rquest, y los revisores estan deacuerdo con la implementación de dos de ellas pero hay una que no está correcta y los revisores no aprueban esa solución. De hecho puede ser que este problema se prolongue en el tiempo, esperando decisiones de los Business Analyst, Arquitectos o desarrolladores, por lo que durante todo este tiempo las demás soluciones se que sí estaban aprobadas, no quedarán integradas en la rama de desarrollo, pudiendo así impedir el desarrollo de más tareas dependientes de las mismas.

En su lugar, presentar tres Pull Requests independientes que se ocupan, respectivamente de su tarea hubieran conseguido añadir la base del código de las soluciones a la rama de desarrollo, liberando así la posible dependencia de otras tareas futuras y dejando centrada la discusión de la tarea conflictiva en un sólo Pull Request.

3. El tamaño de las líneas de código también importa.

En efecto el tamaño de línea  en el código también importa, no sólo para que los revisores tengan mayor facilidad cuando se dispongan a revisarlo si no que, también les permita comparar con una división vertical de pantalla los cambios realizados. Para ello existen herramientas como Stash, GitHub, BitBucket, etc muy apropiadas para ello.

Además de ayudar a los revisores, añadirá más facilidad a los mismor realizadores de los cambios a resolver conflictos con otras ramas cuando se presenten, pues al igual que cuando se comparan soluciones para revisar los Pull Request, los conflictos se resolverán también con herramientas de comparación de archivos los cuales usan división vertical para ello, ayudando así un tamaño de líneas menor.

4. Evita reformatear líneas

A veces dependiendo la forma de programación cada desarrollador puede tener la tentación de cambiar el formato del código existente para adaptarse a «su» estilo. Por favor ABSTENERSE.

El motivo es de libro, cada byte que se cambia en el código fuente se muestra en las vistas de cambios en el Pull Request dificultando y aumentando el tamaño del Pull Request sin añadir funcionalidad alguna. Además el tamaño en disco, servidores de cambios y tiempo empleado en la revisión aumentan.
Si realmente se necesita reformatear código ya se para eliminar espacio en blanco, mover código dentro de los archivos, cambiar el formato o hacer otros cambios de estilo en el código, por favor, hágalo en un Pull Request propio que hace sólo eso así queará identificado y aislado de los Pull Request que añaden funcionalidad extra al código.

5. Asegúrate que tu código compila.

Es imperativo para la colaboración en un proyecto que todo Pull Request que se comience, CONTENGA CÓDIGO QUE COMPILE y que por lo tanto los revisores y testers puedan siempre probar el estado del mismo y la funcionalidad añadida. Dicho esto también es importante para no romper una posible integración contínua implantada en la compañía.

Por eso antes de presentar un Pull Request, es necesario construirlo en su propia máquina. Es cierto que a veces probar sólo en la máquina local no es particularmente útil, pero es una medición mínima. Si no funciona en su máquina, es poco probable que funcione en otras máquinas también ¿no crees?.

Cuidado con los avisos del compilador. Ellos no pueden impedir la compilación, por lo que no se dé cuenta si no se ven de forma explícita. Sin embargo, si su Pull Request provoca más advertencias del compilador de las ya existentes, un revisor puede rechazarla.

Si el proyecto tiene un script de construcción, intente ejecutar eso, y sólo enviar su Pull Request si el script concluye satisfactoriamente.

6. Asegúrate que los Unit Tests pasan satisfactoriamente.

También sería ideal que, antes de abrir el Pull Request (o durante el transcurso del mismo) se deban ejecutar todos los Unit Test que existan y asegurarnos que ninguno de ellos falla por culpa de las modificaciones de nuestro trabajo o que quizás se necesiten cambiar debido a la nueva funcionalida que hemos añadido.

7. Añade Unit Test que cubran tu código.

Como hemos comentado anteriormente, todos los unit tests deben pasar, pero además debemos asegurarnos que nuestro nuevo código está cubierto al 100% (o el porcentaje mínimo que consideréis), pero siempre tener en cuenta que será necesario asegurarnos que nuestro Pull Request cumple con la nueva funcionalidad que se desea añadir y ¿qué mejor forma que con Unit Test?

8. Documenta tus razonamientos.

Como sabemos el código auto-documentado raramente se da. Sí, los comentarios en el código parecen parches o excusas de un código enrevesado y es preferible operaciones bien nombradas, tipos y valores bien definidos a comentarios pero a veces, se toman decisiones durante el tiempo de desarrollo las cuales conviene dejar documentadas, para ello la mejor decisión es documentar el por qué tu escribiste el código de esa forma, no lo que hace.
Para la documentación de código existen varias alternativas y sitios posibles, esta es la lista y mis preferencias:

• Código auto-documentado: Tu código debe ser como un libro el cual se pueda leer de principio a fin sin ninguna duda de lo que hace.
• Comentarios en código: Si no puede hacer que el código suficientemente auto-documentado, deberemos añadir un comentario al código código. Al menos, el comentario se ubica con el código, por lo que incluso en el improbable caso de que se decida cambiar el sistema de control de versiones, el comentario aún se conserva.
• Mensajes en Commit: La mayoría de los sistemas de control de versiones dan la oportunidad de escribir un mensaje de confirmación. La mayoría de las personas no se molestan en poner otra cosa que no sea un mínimo en estos, pero se puede documentar sus razonamientos aquí también. A veces, tendrá que explicar por qué estás haciendo las cosas en un orden determinado y que no encaja bien en los comentarios de código, pero es una buena opción para un mensaje de commit. Siempre que se mantenga con el mismo sistema de control de versiones, preservan estos mensajes, pero si se cambia, lo más probable  es que se pierdan si no se hace una migración bien hecha.
• Mensajes en Pull Request: En raras ocasiones, es posible que se encuentre en una situación en la que ninguna de las opciones anteriores son apropiadas. En los sitemas que soportan Pull Requests tales como GitHub o Stash, también se puede añadir mensajes personalizado. Estos mensajes se conservan en estos sistemas pero si se muda de ejemplo a CodePlex de GitHub, se perderán. Estos mensajes deben ser mensajes para los revisores, para explicar cosas puntuales pero no todo el código ni nada funcional, pues para eso existen aplicaciones de gestión de tareas como Jira.

Recuerda que no es necesario explicar lo obvio, pero ten en cuenta que es obvio para ti hoy pero puede no ser obvio para cualquier otra persona, o para ti mismo en tres meses.

9. Escribe adecuadamente

El código no es solo algo que compila, también es algo compartido y leído por mas personas, por lo que debemos en la medida de lo posible, cumplir con las reglas de ortografía.

Por favor revise la ortografía, la gramática y la puntuación en cualquier lugar donde documente el código (ver punto 8). Si no lo hace, éste será más difícil de entender, y su revisor es un ser humano, usted decide…

10. Evitar sobrecarga de comentarios

A veces un Pull Request puede traer consigo un sinfín de comentarios, los cuales a veces lejos de ayudar a autor del código podrían hacerle bloquearse o replantearse la solución existente. En vez de esto existen varios trucos que podemos aplicar para reducir el número de comentarios:

• Si existen muchas diferencias entre la solución propuesta y lo que se esperaba por la parte del revisor, es más eficiente tratar de resolverlo de manera presencial si se puede u online. Si sobrecargamos el Pull Request con comentarios para cambiar el 90% del código, éste sera inmanejable.
• A veces se puede duplicar el mismo cometario en varios archivos, en ese caso es recomendable crear un sólo comentario indicando todos los archivos o lugares donde cambiarlo.
• Si un revisor considera que el nombre de la variable no es descriptiva y él tiene un nombre mejor a veces y previa aceptación del autor del Pull Request y para evitar varios cambios de contexto de los desarrolladores, el mismo revisor podría cambiar el nombre de la variable por la más apropiada y evitar así comentarios que no añaden funcionalidad al código.
• Abtenerse en comentar espacios de más, saltos de línea que sobran o «errores» similares, no aportan nada al código e implican tiempo al revisor, tiempo al autor y cambios de contexto si se desean «arreglar». De hecho el único comentario que debería haber es para que se use un autocorrector que contenga el IDE usado, que debe estar configurado con las reglas deseadas.

11. Punto extra: Idioma
Ya que casi el 100% de los lenguajes de programación utilizan el inglés y por lo tanto nuestro código es coherente con ello, los PRs debería serlo igual, quién sabe si en un futuro parte de nuestros compañeros no usan nuestro mismo idioma!
Seguro que muchos de vosotros tenéis otros puntos de vista u otros puntos también importantes, así que cualquier comentario ayudará a añadirlos a esta lista. ¡Gracias a todos!