Atlasingeniería

Resumen para el parcial

Code review en la práctica

Qué comenta de verdad un equipo cuando revisa un cambio, ordenado por patrón: un año de revisiones sobre una aplicación con varias marcas, países e idiomas, contadas sin código ni nombres.

Armado con los posts publicados al 20 de septiembre de 2026.

Cómo se ve una review de verdad

Qué se comenta realmente en un code review

Idea clave
El code review real no discute algoritmos: discute contexto. Son preguntas cortas sobre lo que el diff no muestra —qué se borró, para qué marca, con qué valor por defecto, si eso ya existía—. Escribir el cambio pensando en esas preguntas es lo que convierte una review de once comentarios en una de dos.

Qué encuentra un revisor automático y qué sólo ve un humano

Idea clave
La herramienta responde si está bien escrito; la persona, si es lo que había que hacer. Todo comentario humano que se pueda expresar como regla es una regla que falta, y cada uno que se automatiza libera atención para lo único que no se puede automatizar: el contexto.

Responder, discutir y cerrar comentarios sin trabarse

Idea clave
Casi todo hilo trabado es un desacuerdo de tipo, no de fondo: una sugerencia leída como bloqueante. Etiquetá el tipo al comentar, respondé con información que el otro no tenía, aceptá rápido lo barato y, si la segunda vuelta no agregó nada, hablalo y volvé con la conclusión escrita.

Cuando la review descubre que hay que hablar con otro equipo

Idea clave
Si el comentario no se arregla cambiando el código, el problema está en un límite entre equipos. Aislá la adaptación en un solo lugar, dejá escrito a qué decisión espera, abrí la conversación afuera con el costo concreto, y entregá el cambio igual. Lo que no queda en un esquema o en un test de contrato se vuelve a discutir.

Los comentarios que vuelven siempre

Valores sueltos que deberían ser constantes o enums

Idea clave
El valor suelto convierte un error de compilación en un error de comportamiento. Extraelo si se repite, si es un concepto del dominio o si el número no se explica solo; declaralo una sola vez y compartilo entre las dos puntas. Y si alguien de negocio puede querer cambiarlo, no era una constante: era configuración.

Nombres negados, ambiguos o atados a un experimento

Idea clave
Un nombre se escribe una vez y se lee cientos. En positivo para no obligar a negar; con sustancia, unidad y formato para no adivinar; y sin arrastrar el nombre de un experimento que ya terminó. Si el nombre honesto suena feo, no es problema del nombre: la función hace dos cosas.

El helper ya existía: buscar antes de escribir

Idea clave
La copia nueva no está mal: está incompleta. No cubre los casos raros que la original fue absorbiendo, y cuando alguien arregle un bug, sólo lo va a arreglar en una. Buscá por el rastro que deja la salida y no por el nombre, extendé en vez de copiar, y unificá sólo lo que tenga que cambiar junto.

Cambios que no tenían nada que ver con el ticket

Idea clave
Un pull request mezclado no se puede revisar a fondo ni revertir con precisión. Entra lo que el cambio necesita para existir, lo que quedó muerto por él y sus tests; lo demás va aparte, aunque ya esté escrito. Si el refactor es necesario, va primero y solo, con los tests en verde como prueba de que no cambió nada.

Ruido del editor: comillas, comas y reindentados que tapan el diff

Idea clave
Cada línea de ruido en un diff se paga con atención del revisor, que es el recurso más escaso del proceso. El formato se resuelve con una herramienta y una verificación automática, no con disciplina; y cuando haya que reformatear en serio, que sea un cambio solo y registrado para que el historial siga sirviendo.

Calcular dos veces lo mismo y otras condiciones difíciles de leer

Idea clave
El cálculo repetido no es un problema de rendimiento sino de verdad: dos copias que pueden dejar de coincidir. Y una condición larga, un ternario anidado o una cadena de comprobaciones esconden siempre lo mismo: un caso sobre el que nadie decidió qué hacer. Nombrar las partes es lo que lo hace visible.

Cambios de comportamiento que el diff no muestra

Borrar código que todavía le servía a alguien

Idea clave
Antes de borrar, cambiá la pregunta: no «¿esto se usa?», sino «¿para quién deja de pasar esto?». La respuesta casi nunca está en el editor: está en la lista de marcas, de países, de experimentos y de ambientes. Y cuando la respuesta es «para nadie», eso también se escribe en la descripción del pull request.

Defaults silenciosos: el valor «por las dudas» que tapa un bug

Idea clave
Un default convierte una falla ruidosa en una mentira consistente. Antes de escribir || algo, preguntate qué pasa si ese algo se muestra un millón de veces: si la respuesta es «mostramos un producto equivocado» o «prometemos puntos que no existen», la opción correcta no es otro valor por defecto, es no mostrar nada y dejar rastro de que el dato faltó.

Condiciones de un A/B incompletas y banderas que ya ganaron

Idea clave
Un experimento tiene cuatro estados, no dos: variante, control, fuera y sin respuesta. Escribí la condición por la afirmativa para que lo imprevisto caiga en el comportamiento conocido, no bloquees la pantalla esperando la asignación, y considerá que el experimento termina recién cuando se borró la rama perdedora.

Multimarca y multipaís: un identificador no es el otro

Idea clave
Marca, país, idioma, moneda y mercado son cinco cosas distintas que casi siempre coinciden. Esa coincidencia es la trampa: el código que las confunde funciona hasta la primera excepción y después falla en todos lados a la vez. Tipos distintos, una tabla de configuración única, nada derivado, y el caso desconocido que no cae a la marca principal.

Textos, traducciones y encoding: el idioma también es código

Idea clave
El texto es código con reglas propias. No se escribe en la pantalla, no se arma concatenando fragmentos traducidos, la falta de una traducción es una decisión de producto que hay que tomar y registrar, y la codificación se declara igual en toda la cadena. Y probá la interfaz con el idioma más largo antes de darla por terminada.

Robustez donde el usuario es impredecible

Nulos, listas vacías y respuestas que no llegan

Idea clave
Vacío, cargando y error son tres estados distintos y se ven iguales si nadie los distingue. Validá en el borde para que el tipo sea cierto adentro, poné tiempo límite a lo que puede no responder, dejá que lo accesorio desaparezca sin arrastrar la pantalla, y registrá siempre lo que se oculta en silencio.

Cuando el navegador bloquea el almacenamiento local

Idea clave
Acceder al almacenamiento del navegador es una operación que puede fallar, y si está en el arranque, se lleva la pantalla. Encapsulá el acceso con captura de errores y validación de forma, guardá conveniencias y nunca la verdad, y tratá lo que no entendés como ausente.

Scripts de terceros que cargan antes, después o nunca

Idea clave
Un script de terceros puede no llegar, llegar tarde o bloquear, y corre con tus mismos permisos. Nunca asumas su objeto global, cargalo sin bloquear, encolá lo que pasó antes, envolvelo en una capa propia y mantené un dueño por cada uno. Si una funcionalidad no existe sin el tercero, ese tercero es parte de tu disponibilidad.

Efectos ya aplicados cuando el proceso falla a la mitad

Idea clave
Sin transacción, el orden de las operaciones es el diseño. Lo reversible primero, lo irreversible al final y una sola vez, clave de idempotencia para que el reintento no duplique, y compensaciones escritas de antemano para lo que ya ocurrió. Y nunca confirmarle al usuario algo antes de que sea cierto.

Tests que pasan sin probar nada

El test que pasa sin probar nada

Idea clave
Verde no significa probado. Un mock sin configurar, un catch que absorbe todo y un nombre que promete más de lo que afirma alcanzan para tener una suite completa que no detecta nada. La única prueba de que un test sirve es haberlo visto en rojo por el motivo correcto.

Tests atados al reloj y a la fecha de hoy

Idea clave
El tiempo es una dependencia y se inyecta como cualquier otra. Pasá el momento de referencia, escribí los valores esperados a mano en vez de recalcularlos, probá los extremos exactos del rango, y guardá en tiempo universal convirtiendo sólo al mostrar.

Todo el flujo en un solo caso: cuando falla no se sabe dónde

Idea clave
El nombre de un test es el mensaje de error que vas a leer en rojo. Un escenario compartido y un caso por comportamiento hacen que el fallo localice solo; mezclar reglas distintas en un solo caso obliga a arreglarlas de a una, con una vuelta de pipeline cada vez.

Esperas mágicas y timeouts sin explicación

Idea clave
Una espera fija dice «confío en que ya pasó»; una espera por condición dice qué se está esperando. El test inestable es información sobre una carrera real, no ruido que se tapa con medio segundo, y en producción la misma apuesta contra el reloj falla justo el día que el sistema está lento.

atlas.matiascaliz.com.ar/materias/code-review-en-la-practica — si algo de acá no se entiende solo, el post completo lo explica.