Pull Request enormes: ¿Cómo evitarlos?

Asumamos que estás trabajando en un proyecto donde asiduamente se hacen code reviews. Esto es algo muy pero muy bueno; ¡bien por el equipo! No obstante, detrás de esta práctica que produce que nuestro código sea mejorable (gracias a la detección temprana de fallas, la posibilidad de pulir sutilezas y el aprendizaje de parte de la persona que brinda su tiempo en la evaluación, además de los consejos otorgados sobre diversos tópicos sobre el tema), también ocurren situaciones poco felices como la que trataremos en este post. Estamos hablando de lo que ocurre en casi todos los proyectos: la situación del Pull Request de la muerte.

¿PR de la muerte?

Muchas veces no nos sentamos a pensar en esa persona que, con sus problemas y sus tareas pendientes, asume la gesta de revisarnos el código. He aquí que le dejamos nuestra ofrenda: entre 50 y 60 archivos de lógica intrincada, de idas y vueltas, de tests con infinitos casos.

Cuando cualquier desarrollador se enfrenta a esto, pueden ocurrir varias cosas, por ejemplo, nos van a odiar mucho, y nos dirán: “¿Por qué?, ¿por qué?” y eventualmente lo revisarán (o no...), o puede que con indiferencia esté en stand by nuestro pequeño engendro hasta que un alma con gusto por los sacrificios tome la tarea que, por lo ardua, aún aplicando la mejor voluntad, no llegue a evaluar todo lo requerido.

¿Realmente era necesario ese exabrupto de creación, y por eso debíamos mandarlo todo junto? Casi nunca lo es.

La realidad es que lo hacemos sin darnos cuenta. Seguimos avanzando en nuestra implementación y, de tan emocionados que estábamos (quizás durante días), en un buen momento nos dimos cuenta de que era hora de que alguien revise tamaña hazaña y ahí vamos, orgullosos, pidiendo que alguien lo mire, lo evalúe y nos permita seguir nuestro paso a producción. La pobre persona encargada de evaluar el código beberá lentamente su café, tomará tal épica tarea y revisará por encima nuestro código y, si el viento sopla a favor, muy posiblemente sin una revisión a conciencia, el código viajará directo a donde deba ser mergeado, con algún que otro problemita a resolver por nuestro yo del futuro, si es que no tiene la mala suerte nuestro desdichado reviewer de, en alguna ocasión, tener entre sus tareas retomar el código que tan rápido aprobó (y ahí nuevamente se acordará de nosotros).

Una posible solución

Este post es para todos los que usamos git. En él describo una técnica, obtenida en parte gracias a un ida y vuelta de la empiria, que a mi entender me acerca bastante a la situación de lograr PR cortos, concisos, con nombres de branches relacionados con lo que estamos trabajando, y donde logramos que las reviews sean rápidas, focalizadas y, por ende, logrando de manera más exitosa una evaluación “al detalle”.

El secreto está en cuándo crear el branch

Cuando empezamos a trabajar sobre un nuevo feature, y ya tenemos medianamente claro qué es lo que deseamos implementar, en general lo que hacemos es crear un nuevo branch y a partir de ahí empezamos a hacer nuestros tests. Asumo que usamos TDD, ¿no? ¡Bien, felicitaciones por eso! (y si no lo hacemos; ya di mi felicitación y no tiene devolución. ¡Ahora no nos queda otra más que empezar a usarlo!). Una de las cosas que se sugiere que hagamos es inicialmente no poner nombre a los tests. Un “test01” (ó cualquier nombre que no sea relevante)  es quizás suficiente, ya que al principio no sabemos en su totalidad cuál será el enfoque del test. Luego llegará el tiempo en el que ese test se transforme y puede que incluso sean más, o simplemente finalmente tengamos que solamente cambiar su nombre por uno correcto. Si tomamos esta práctica como buena (les aseguro que lo es), llevémoslo a un nivel superior con la creación de branches para nuevas funcionalidades.

Poner buenos nombres a los branches ayuda a hacer más efectiva la comunicación de lo que estamos realizando. Pero al principio del desarrollo no tenemos bien claro el foco del futuro PR, y más aún si queremos hacer PR cortos y concisos. Muchas veces ni siquiera sabemos si lo que queremos implementar o corregir implica simplemente una línea de código ó un refactor más grande (y si quisiéramos corregir un poco de deuda técnica, la escala de cambios podría ser mucho más grande). El alcance de la tarea a efectuar es algo que usualmente es un misterio que se resuelve luego de tener una visión acabada e implementada del tema a atacar. La solución entonces es inicialmente sacarnos el peso de asignar el nombre del branch hasta no tener partes definidas de implementación que sean factibles de ser revisadas.

Entonces podemos encarar dos formas el trabajo:

A.

1.- No crear branch al principio, tantear el terreno:

Al inicio, trabajar sobre master hasta lograr desarrollar algo conciso (sin realizar ningún commit, este paso lo realizaremos más adelante) que consideremos manejable y revisable. Incluso podemos seguir trabajando, pero ya habremos detectado una primera “unidad” de código para mandar a review.

2.- Una vez que encontramos esa unidad lo que nos queda es:

• git stash => guardamos nuestros cambios en memoria para pasarlos a nuevo branch.
  • Una posibilidad en esta etapa es ir iterando sobre los cambios, inicialmente trabajando sobre un commit con nombre wip (Work in Progress), e ir haciendo “amend” sobre los mismos hasta lograr un estado final, para luego deshacer este commit y continuar con los pasos propuestos a continuación.
• Creamos el branch con el nombre específico relacionado a esa unidad.
• Creamos el branch con el nombre correcto => git checkout -b “mi_pr_para_la_unidad”.
• Volcamos lo que teníamos en memoria a este nuevo branch => git stash pop.
• Solamente haremos commit de los archivos relacionados, dejando el resto para lo que estamos continuando.
• Mandar a PR y continuar con esta técnica hasta detectar una nueva unidad.

B.

1.- Crear branch al principio, pero con un nombre por ahora “sandbox”:

Si al inicio no nos sentimos seguros de trabajar sobre master por miedo a cometer un error, entonces:

• Creamos un branch con cualquier nombre desde master (yo utilizo “feature-sandbox”, puede ser el que gusten).
• Trabajamos sobre nuestra funcionalidad, aunque atención: deberemos ser conscientes de cuándo detener nuestro desarrollo, ó luego tendremos que plantear algún punto conceptual de separación de lo que queremos cerrar y con qué deseamos seguir trabajando.
• Una vez que obtuvimos una unidad válida de review¹, hacemos un commit identificando dicho código. El resto de ese código que no iría para review, lo mandamos a stash.
• Renombramos el branch con el nombre referido a la unidad en concreto,
  • git branch -m “feature-sandbox” “Mi_pr_para_la_unidad”
• Ahora volvemos al branch “Mi_pr_para_la_unidad” y lo mandamos a review.
• Abrimos un nuevo branch “feature-sandbox” desde “Mi_pr_para_la_unidad” y sacamos del stash (git stash pop) lo que estábamos trabajando hasta ese momento.

Como siempre, debemos tener en cuenta antes de mandar a review que el branch a revisar tenga lo último de master actualizado y con todos los test pasando en “verde”.

Vamos a contar rápidamente con estos beneficios:

• El flujo de review es mucho más rápido, con mayor frecuencia haciendo merge de lo que estemos trabajando contra versiones ya probadas.
• Realizaremos correcciones de mayor cantidad de cosas más tempranamente.
• Lograremos mayor visibilidad para el resto de los compañeros de trabajo sobre las tareas que realizamos y la evolución de lo que estemos involucrados.

En lo particular, a mi me resulta mejor la segunda opción (bash aliases² mediante), ya que la primera me parece un poco más propensa a errores. No obstante, cualquiera de las dos son válidas. Como todo, lo importante a tener en mente es el objetivo de lograr PR cortos, para lograr un mayor beneficio de las reviews.

1. Una unidad válida de review es una implementación que es considerada como una modificación, granular, que realizamos en el código, la cual es lo suficientemente acabada conceptualmente para ser revisada, sin por ello producir fallas en el código pre-existente. Ej; Si estamos realizando una clase que efectúa las operaciones matemáticas básicas, implementar una operación, como ser la división (con sus casos especiales, como la división por 0) podría ser llevado finalmente a un PR como una unidad válida de review.

2. Nota: Les  dejo mis “bash aliases” para quien los desee replicar y empezar a aplicar la técnica propuesta:

Aliases

• alias bsan="git checkout -b feature-sandbox"
• alias msan="gitb -m feature-sandbox "
• En este como parámetro al final el nombre del branch final que desean darle: ej : msan “feature_increible_sobre_login”
• alias dsan="delbranch feature-sandbox"
• alias csan="git checkout feature-sandbox"
• alias cmas="gitck master; git pull"
• alias mmas="git merge master"

Funciones

• Crear el branch remoto:

function gpush {
    if [ ! $1  ] ; then
        git push -u origin $(git rev-parse --abbrev-ref HEAD)
    else
        git push -u origin $1
    fi
}

• Agregar Underscores a un nombre con espacios

function wusc {
    sed 's/ /_/g' <<< $1
}

• msan $(wusc “nombre feliz”) => Como  resultado crearán desde un branch llamado feature sandbox, ahora estarán en el branch “nombre_feliz”