Guía completa para una Pull Request perfecta
El primer paso para un buen ambiente en las revisiones de código.
Solicitar cambios porque patata.
Los commits de esta edición:
feat: revisiones de código para humanos
fix: LaLiga, la NASA y el nuevo iPhone
chore: nueva muestra libro + mensaje eliminado
refactor: sacar dinero infinito
¡Dale! (Zelda, dale)
Tema principal
Hace tiempo me daba miedo abrir una Pull Request (PR).
Tenía que esperar revisores de otro equipo, siempre se retrasaban y ponían pegas a cualquier gilipollez. Que valoraran mi trabajo tan minuciosamente me ponía nervioso.
Si en un sprint de 2 semanas hacemos 3 o 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.
Pero por aquel entonces no sabía que el problema no era mi código, sino mis compañeros.
A día de hoy, para mí las PRs son un lugar de aprendizaje, tanto para juniors como para seniors. En los equipos Frontend que lidero (actuales y pasados) todo el equipo tiene que aprobar los cambios.
(No te asustes, eran 3 personas para aprobar como mucho y había una coordinación brutal. Nada de esperar horas para el verde.)
De esta manera los más novatos ven 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.
Una PR debe estar bien detallada y ser un debate entre todo el equipo.
Lo vemos en detalle:
Título y descripción
Hay días en los que hay muchas PRs para revisar, sobre todo en equipos de 4+ 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.
Y si tiene un ticket JIRA asociado, inclúyelo por favorrrrr.
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 developers 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 a simple vista qué hace. Si queremos unificar varias tareas para mejorar la performance nos tocará escribir el for de toda la vida, eso sí.
Que sea un debate abierto
Para mí, este es el punto más importante y el objetivo final de las PRs.
Mejor si 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, porque el código seguirá ahí cuando salgamos del proyecto. Y en algún momento cambiará o desaparecerá.
Seamos profesionales y no héroes. Al final, todos estamos en el mismo bando.
Comentar, preguntar y sugerir siempre
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 fuera necesario
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 tu propia 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 juniors como de seniors.
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.
La semana que viene hablaré de problemas habituales en las PR y cómo solucionarlos.
Y en tu equipo, ¿qué?
¿Todo bien o repartimos puños en la nuez?
📰 Noticias (por si no te has enterado)
🎨 He descubierto que Paint tiene IA - Puedes generar imágenes con tu cuenta de Microsoft. Dudo que alguien quiera hacerlo. (+info)
Ⓜ️ Meta apuesta por los robots humanoides avanzados - Dando bandazos después del fiasco del metaverso y de despedir al 5% de su plantilla hace un mes. (+info)
🤑 $LIBRA: ¿criptoestafa o todo ok? - Tiene cierto retrogusto a lo que pasó con GameStop y el grupo de inversores de reddit. Bueno, grupo de inversores o trols, no sabría decir. (info de Wikipedia, para no caer en un bando u otro, y un vídeo de midudev)
⚽️ LaLiga sigue empeñada en bloquear IPs. No está nada claro que pueda hacerlo legalmente - Continúa la movida entre LaLiga y Cloudfare. Si no te has enterado de la movida, LaLiga pidió a Telefónica que bloqueara IPs su acceso a Cloudfare, pues detectaron señales emitiendo fútbol pirata. Pero también había miles de usuarios intentando acceder a otros servicios online alojados en Cloudfare (como Github). ¿Ataque a la libertad de Internet? (+info)
🚀 Las 10 reglas de la NASA para su desarrollo de software - Interesante lectura diagonal o de los títulos para ver cómo programan en la NASA. (+info)
🏗️ Arquitecto de software: ¿una figura mitológica? - Una explicación de la evolución del rol, sus competencias y sus responsabilidades. (+info)
Ah, y han anunciado el iPhone 16e, pero eso te lo dejo investigar a ti.
🪳 Otra semana sobreviviendo al Software
El domingo pedí una nueva versión de prueba del libro; fecha de entrega estimada, 27 de febrero. OMG! Otros 11 días de espera. Por lo menos me da tiempo a ir haciendo un par de webs para el libro. Cositas que encontrarás dentro para extender la experiencia de Junior, Senior, Lead (así se titula).
También me hice unas fotos en condiciones. Mi primera sesión fotográfica para tener alguna foto decente para el libro y para dosieres de charlas. Lo que me recuerda que mandé 2 propuestas de charlas a Codemotion.
¡Ah! Compré el Mario Wonder por Wallapop y estoy algo viciado 😅
En la oficina, Petunia y Juanjito se han pasado jiji-jajá toda la semana. Incluso en las videollamadas se les veía sonriendo y tecleando, como si se estuvieran mandando mensajitos en paralelo.
El pobre Izan vuelve a estar distante, pero al menos me ha hecho un montón de preguntas de JavaScript y está muy interesado en mejorar. Supongo que para ponerse al nivel de su enemigo sentimental.
Justo anoche me quedé hasta tarde programando una landing page para el libro (10 de marzo, fecha lanzamiento), cuando de repente me llegó un wasap al móvil.
Eran las 3:12h.
Mensaje de Izan al grupo del curro.
La notificación decía: “A ver Petu tia q tenteres de una vez q lo q quier…”
Entré volando a verlo, pero solo encontré “Se eliminó este mensaje”.
WTF.
👨🏻🏫 Practica un poco, bro / sis / sib(ling)
Por último, te dejo un ejercicio rápido para que practiques un poco. Si lo resuelves a mano, con papel y boli, mejor. (Tienes los beneficios aquí)
// Crea una clase para controlar un cajero automático:
// Se puede comprobar saldo y sacar dinero, después de
// autenticarse.
// Ten en cuenta que el cajero tiene dinero limitado.
class ATM {
constructor() { /* ¿Necesitas propiedades aquí? */ }
}
// Ejemplo de uso:
const atm = new ATM();
atm.auth('1234'); // Autenticación del cliente
atm.checkBalance(); // Muestra saldo del cliente
atm.withdraw(200); // Retira dinero y actualiza saldo
// Un punto extra:
// ¿Cómo se podría añadir funcionalidad para ingresar dinero?
atm.deposit(300); // Deposita dinero y actualiza saldo
Por cierto, no he programado la solución, asume los datos que consideres.
Y eso es todo.
Una semana más sobreviviendo al Software 💻 💻 💻 💻 💻 💻
Chao, pescao 🐠
Llega la etapa importante de Izan, la de ponerse mazas en el gimnasio del software para sorprender a Petunia. Dentro de poco se dará cuenta que lo que Petunia admira en Juanjito son sus soft skills....