Lo que espero de una Pull Request
La forma antes que el fondo.
A menudo, una Pull Request (PR) puede resultar intimidatoria: el resto del equipo va a valorar nuestro trabajo y es normal ponernos nerviosos.
Si en un sprint de 2 semanas hacemos 3–4 PRs como mínimo, a lo largo de un año podemos hacer alrededor de 90. Son demasiados momentos para estar nervioso.
No merece la pena.
Para mí, las PR son un lugar de aprendizaje, tanto para júniors como para séniors. En los equipos Frontend que lidero (actuales y pasados) todo el equipo tiene que aprobar los cambios.
De esta manera los más novatos verán cómo programan los más experimentados y, a su vez, estos podrán recibir puntos de vista más “simples” o ingenuos que tal vez no hayan barajado.
La forma antes que el fondo
Más allá de que el código funcione, para mí una Pull Request debe estar bien detallada y ser un debate entre todo el equipo.
Me explico:
Título y descripción
Hay días en los que hay muchas PRs para revisar, sobre todo en equipos de más de 3 personas y cerca del final del sprint (desgraciadamente). Por eso es necesario explicar qué hace nuestro código y cuál es la tarea o ticket que resuelve.
¿Se trata de una feature, de un (hot) fix o de un refactor?
Explica detalladamente, pero de forma escueta, qué hace está PR.
- Se actualiza la url del endpoint.
- Se elimina código en desuso.
- Se añaden condiciones a los filtros de todas las tablas.
Imágenes
Siempre que hagamos cambios visuales en componentes o vistas deberíamos mostrar cómo se ve la aplicación antes y después de nuestros cambios. Ya sea al hacer un componente nuevo, al añadir una sección a una modal o cambiar el color principal del tema de la aplicación.
Muchas veces desarrollamos y maquetamos con datos mock y cuando enganchamos el servicio real no nos percatamos de overflows y scrolls que antes no estaban.
O simplemente hemos pasado por alto algún punto del ticket.
Si ponemos imágenes en la PR puede que los compañeros vean que falta algo.
Código que lo entienda una persona
A muchos programadores y programadoras les gusta que el código ocupe pocas líneas.
No es mi caso.
Me gusta que el código ocupe lo mínimo, sí, pero me gusta más entenderlo a simple vista.
- Si hay que poner un if en 3 líneas, se pone.
- Los nombres de variables y funciones deberían ser lo más específico posible, aunque sean más largos de lo normal.
- No debería haber ternarios encadenados (ternarios dentro de ternarios), ya que su lectura se complica y se dificulta la comprensión.
- Mejor usar funciones específicas de los Array en vez de bucles normales, para entender mejor qué hacemos. Si queremos unificar tareas para mejorar la performance (filter + map + some) nos tocará escribir el for de toda la vida, eso sí.
Si te gustan los one-liners, aquí te dejo 3 de mis favoritos:
Que sea un debate abierto
Para mí, el punto más importante y el objetivo final de las PR.
Mejor si las PRs son un debate constructivo en el que todos participan y nadie se ofende. Comenta con respeto, ayuda al equipo y reconoce tus errores.
Sugiere cambios, pero no ordenes.
Comenta sin ridiculizar a nadie. Todos hacemos nuestro trabajo lo mejor que podemos.
Dejemos los egos a un lado. El código seguirá ahí cuando salgamos del proyecto o la empresa, mutará y mutará.
Seamos profesionales y no héroes. Al final, todos estamos en el mismo bando.
Comentar, preguntar y sugerir hasta el infinito
Las PRs más sencillas no necesitan comentarios, pero si alguien aprueba TODAS las PRs y no comenta nunca… golpe de remo. 🏏
No comentar nunca da la sensación de que esa persona no está mirando las PRs con atención. O no las está mirando, directamente.
¿Eres júnior? Puede que no tengas nada que sugerir al resto, pero seguro que tienes alguna duda acerca del funcionamiento de parte del código. Pregunta por qué han elegido un map frente a un bucle for normal o por qué un ternario y no un if.
¿Eres sénior? Seguro que puedes mejorar la performance de 3 bucles for anidados, convertir un bloque complejo a reduce o sugerir una desesctructuración de variables en vez de pasar la referencia completa.
Y si no tienes nada que añadir, puedes aprobar la PR y dejar un simple “Buen trabajo” o “Qué interesante, yo hubiera usado un filter”.
La comunicación favorece a que el equipo esté más unido. Y un equipo unido trabaja mejor.
Comentarios explicativos si alguna parte lo necesita
No solo los revisores pueden poner comentarios. A veces, alguna funcionalidad puede resultar enrevesada o es tan diferente al resto de la aplicación que está bien explicar por qué se controla de esa manera.
Abre la PR y añade comentarios en las partes que consideres complicadas, en las que se necesite mayor atención o en las que pienses que se puede hacer mejor, pero solo has encontrado esa solución.
Igual que puedes comentar/preguntar en las PRs de los demás, en las tuyas también puedes participar antes de que el resto entre a verlas.
Las Pull Request, bajo mi punto de vista, son un espacio para el debate y el aprendizaje, tanto de júniors como de séniors.
Que no te dé miedo abrir PRs, preguntar o sugerir cambios.
Hagamos de ese espacio un lugar para todos los públicos y libre de egos.
¿Se me olvida algo?
