Atlasingeniería

Code review en la prácticaCómo se ve una review de verdadTema 4

Cuando la review descubre que hay que hablar con otro equipo

Un comentario que arranca con «esto lo devuelve el backend así» y termina en una reunión con otro equipo. Cómo se reconoce que el problema no es del cambio, y qué hacer para que el pull request no quede esperando una decisión ajena.

Hay un tipo de comentario de review que no se puede resolver cambiando el código: el que revela que el problema está del otro lado de un límite. Un campo que llega con un formato raro, un identificador que significa dos cosas, un endpoint que devuelve null donde debería devolver una lista vacía.

La reacción natural es adaptarse en el cliente y seguir. Es la que más deuda genera, porque la adaptación queda para siempre y el problema original nunca se entera de que existe.

Cómo se reconoce

Lo que se ve en el cambioLo que suele significar
Una función que normaliza la respuesta antes de usarlaEl contrato no está definido o cambia según el caso
Un `if` por cada marca o país sobre un mismo campoEl campo mezcla dos conceptos distintos
Un comentario del tipo «viene así por ahora»Hay una decisión pendiente de otro equipo y nadie la está siguiendo
Cálculos de negocio en el cliente sobre datos crudosFalta un dato derivado que el backend podría entregar resuelto
Los cuatro se pueden arreglar en el cliente, y los cuatro vuelven en el próximo ticket.

El último es el más caro en un producto con varias plataformas. Si el total, el impuesto o la elegibilidad se calculan en el cliente, ese cálculo se va a escribir de nuevo en cada aplicación, y con el tiempo cada una va a dar un número ligeramente distinto.

Qué hacer con el pull request mientras tanto

El error de proceso es dejar el cambio abierto esperando una respuesta que tarda días. El ticket sigue corriendo y la rama envejece.

Cómo separar lo que se puede entregar de lo que hay que negociar

  1. Aislar la adaptación en un solo lugar. Si hay que compensar algo del otro lado, que esté en una función con nombre explícito, no repartido por la vista. Cuando se arregle en el origen, se borra un archivo.
  2. Dejar por qué existe. Este es el caso donde un comentario en el código se justifica: no explica qué hace, explica que es un ajuste temporal y a qué decisión está esperando.
  3. Abrir el tema afuera, con un ticket o un mensaje al equipo dueño, referenciado desde el pull request. Sin eso, la conversación existe sólo en el hilo de la review y muere ahí.
  4. Entregar lo que funciona. El cambio sale con la adaptación acotada; el arreglo de fondo va por su carril.

Cómo se plantea del otro lado

La conversación entre equipos funciona mucho mejor cuando se plantea desde el caso concreto y no desde la crítica al diseño.

Antes de seguir, predecí

Un endpoint devuelve `null` cuando no hay resultados, y el equipo cliente tiene que chequearlo en seis lugares. ¿Qué planteo funciona mejor?

Y hay una pregunta previa que ahorra discusiones: ¿de quién es esa decisión? Muchas veces el equipo dueño del servicio tampoco puede cambiar el formato porque hay otros tres consumidores. Saber eso al principio cambia la propuesta: en vez de pedir un cambio, se pide un campo nuevo.

Que quede escrito en algún lado

Lo que evita repetir la conversación es que el acuerdo viva fuera de las dos cabezas que lo tomaron.

Tres formas, de menos a más sólida

  1. El ticket con la conclusión. Mínimo aceptable: queda buscable.
  2. El esquema como fuente. Si el contrato está declarado —un esquema compartido, tipos generados—, la próxima discrepancia la encuentra el compilador y no una persona en una review.
  3. Un test de contrato. Corre del lado del proveedor y falla si rompe lo que el consumidor espera. Es lo único que impide la regresión seis meses después, cuando nadie se acuerde de la conversación.
Más a fondo · nivel seniorLa ley de Conway en una review

Cuando el mismo tipo de comentario aparece siempre en el mismo borde —“esto tendría que venir resuelto”—, el problema no es de las personas sino de dónde está trazado el límite entre los equipos. Un dato que siempre hay que completar del otro lado suele indicar que la responsabilidad quedó partida en el lugar equivocado. Eso no se arregla en una review, pero las reviews son donde se detecta.

El problema está del otro lado

src/catalog/product-adapter.ts+14−0

Hay 4 problemas en este cambio. Tocá la línea donde creas que está.

@@ -1,6 +1,26 @@
11
22
3
4
5
6
7
8
9
10
11
12
13
14
15
16

Tres adaptaciones razonables. Si ninguna se reporta, cada equipo que consuma esa API va a escribir su propia versión.

Cómo se cruza el límite

Cierre

Autoevaluación

¿Lo entendiste?

En una review aparece que un campo llega con dos significados según la marca. ¿Cuál es el peor camino?
¿Qué evita que el acuerdo entre equipos se pierda en seis meses?