Lección 50 — Revisión de código

Stack: Habilidades · Proyecto: TicketFlow Estado: Publicada — dar y recibir feedback sin que se rompa el equipo Prerrequisito: Lección 49 — Estimar y dividir problemas

Objetivos

1. Ejecutar la revisión con método: qué se revisa (y en qué orden), qué es bloqueante y qué es sugerencia.
2. Escribir comentarios de review que enseñan y no atacan: el criterio, el tono y la pregunta que vence a la orden.
3. Recibir la revisión sin ego: la respuesta al feedback, el desacuerdo con datos, y el checklist del PR del autor.

1. Qué se revisa (y en qué orden)

El review efectivo lee en orden de impacto: (1) el QUÉ (la descripción del PR: ¿la intención está clara? ¿el test demuestra la intención? — la 49: el alcance explícito); (2) el contrato (¿la API/DB/eventos cambian? — los tests de contrato de la 35 y el ADR del 48: el cambio de contrato exige su doc y su versión del 14); (3) la seguridad (la 22: el input del usuario, el authz del 21, el secreto del 23); (4) la corrección (la lógica, los bordes, la concurrencia del 10); (5) los tests (¿prueban el QUÉ? ¿la red tiene agujeros? — la 34); (6) la legibilidad al final (nombres, duplicación — lo que un IDE/linter ya caza no consume tiempo humano: el ruff/black/mypy del 41 automático, el humano en lo humano). El error del review invertido: 40 comentarios de estilo y cero sobre la carrera de datos del ORM (el 09) — el orden protege el impacto.

La lente del proyecto (lo que TicketFlow revisa SIEMPRE): ¿el servicio sigue sin conocer HTTP/infra (24)? ¿el outbox va tras el commit (25)? ¿el lock protege el inventario (10)? ¿el error sale por el handler (26)? ¿el test es de comportamiento (33)? — cinco preguntas que son el ADN del repo: el review que las hace es el que mantiene el sistema.

2. El comentario que enseña: criterio, tono, pregunta

El comentario de review tiene tres formas útiles: el bloqueante (con razón técnica y referencia: "esto rompe el contrato del 26: si el front parsea type, el 409 sin type rompe el checkout — ver test_contract_guard; propongo el handler central"), la sugerencia (nit marcada como tal: nit: el nombre x → seat_refs; el revisor clasifica el peso: el autor sabe qué debe atender), y la pregunta (la forma más poderosa: "¿qué pasa si dos requests llegan a la vez aquí?" — la pregunta que enseña el riesgo SIN la orden; el autor descubre el lock que faltaba y aprende el porqué, no la regla). Y el tono: se comenta el CÓDIGO, no la persona ("este bucle hace N queries" ≠ "tú no sabes SQL") — la regla del 47 (blameless) aplicada al texto del PR.

❌ "Esto está mal, nunca haces queries en un loop."
✅ "Esto lanza 1 query por reserva (el N+1 del 09): con 40 reservas son 83 queries.
    Con prefetch_related son 3 y el test de rendimiento del listado queda en verde.
    ¿Lo hablamos si el caso de uso exige otra forma?"

Las tres piezas del comentario adulto: el hecho (la evidencia), el impacto (qué cuesta), y el camino (o la pregunta que lo abre). El comentario sin camino es juicio; el con camino es ingeniería.

3. La escala del bloqueante: la jerarquía que el proyecto define

No todo comentario iguala. La escala del proyecto (acordada, no inventada por cada review): bloqueante (rompe contrato, seguridad, invariantes de dinero/inventario, o el test de la red), debería (el N+1, el test que falta en la zona crítica: se resuelve en el PR o issue inmediato), nit (estilo/nombre: el autor decide), pregunta (sin posición: abre diálogo). El revisor clasifica y el autor prioriza: el PR con 12 nits y 1 bloqueante no es "12 problemas". Y el review del desacuerdo: si el autor responde con datos y el revisor mantiene: la escalada es al ADR (48) — el review no lo decide el volumen de la voz, lo decide el documento de la decisión (o se crea el ADR nuevo si la discusión lo merece).

4. Recibir la revisión: el ego fuera del loop

El review es del CÓDIGO y para el SISTEMA: el feedback que duele ("esto mezcla capas") es el feedback que enseña. Las reglas del autor: (1) responder TODO (el comentario sin respuesta es el bloqueo silencioso: "hecho", "ok con nit, no lo cambio", o el desacuerdo argumentado); (2) el desacuerdo con datos ("mantengo el loop: son 5 filas fijas y el prefetch complica; el test de rendimiento está en el PR — ¿revisamos el número?") vence al desacuerdo con sentimiento; (3) el PR del autor viene LIMPIO: self-review antes (el diff propio leído con ojos de extraño), los tests en verde (41), el PR pequeño (49: el trozo vertical — el PR de 2000 líneas se revisa con "LGTM" de pánico); (4) el re-review rápido (el revisor que pide cambios responde en horas: el review lento es el impuesto que mata la cadencia del 41).

Y el caso del equipo de 1 (el curso): el self-review estructurado — la checklist del §5 como el revisor virtual, el bot del linter (41) como el revisor mecánico, y el review cruzado del ADR (48: la decisión del PR documentada antes del merge): la revisión de sí mismo es imperfecta pero el MÉTODO la aproxima.

5. La checklist del PR (la del proyecto)

## Checklist del autor (antes de pedir review)
- [ ] El QUÉ está en la descripción con el alcance dentro/fuera (49)
- [ ] Tests de comportamiento nuevos/ajustados (33) y la suite verde (41)
- [ ] El contrato no cambió (35) — o cambió CON su versión (14) y doc (48)
- [ ] Los 5 ADNs del repo: capas (24) · outbox tras commit (25) · lock (10)
      · handler de errores (26) · config del entorno (27)
- [ ] Sin secretos ni PII en fixtures/logs (23)
- [ ] Self-review hecho: leería este diff en un incidente (47) sin preguntar

## Checklist del revisor (en orden de impacto)
- [ ] El QUÉ y el test que lo demuestra
- [ ] Contrato/seguridad/corrección (en ese orden, §1)
- [ ] La clasificación del comentario: bloqueante/debería/nit/pregunta (§3)

La checklist es el contrato social del repo: el review deja de ser personalidad y se vuelve proceso — lo que el PR de un extraño y el self-review del equipo de 1 comparten.

Autoevaluación

1. ¿En qué orden se revisa y por qué el estilo va al FINAL? ¿Cuáles son los 5 ADNs del repo de TicketFlow?
2. ¿Qué tres formas tiene el comentario útil y por qué la pregunta es la más poderosa?
3. La escala bloqueante/debería/nit/pregunta: ¿qué resuelve (el PR de 12 nits) y cómo se escalan los desacuerdos?
4. ¿Qué 4 reglas gobiernan al autor y qué hace el self-review en un equipo de 1?
5. ¿Por qué el PR pequeño (49) es una condición del review y qué cuesta el PR de 2000 líneas?

Continúa con los ejercicios. Las solutions.md solo tras intentarlo.